From 9472ac44019077579ac332b1f1c8890ca6fc9de2 Mon Sep 17 00:00:00 2001
From: DegrangeM <53106394+DegrangeM@users.noreply.github.com>
Date: Thu, 6 May 2021 07:31:11 +0200
Subject: [PATCH] Fix vulnerability (#252)

* Improve and simplify code handling CORS

* Don't execute request when origin not allowed

Fix vulnerability

* Remove webCorsOrigin legacy option

It's confusing (and potentially insecure as removing webCorsOrigin in configuration would still set it to localhost)

* Allow 127.0.0.1 and browser extension if localhost allowed
---
 plugin/config.json |  1 -
 plugin/util.py     |  2 +-
 plugin/web.py      | 59 +++++++++++++++++++++++++++++++---------------
 3 files changed, 41 insertions(+), 21 deletions(-)

diff --git a/plugin/config.json b/plugin/config.json
index 00f2f30..697b96a 100644
--- a/plugin/config.json
+++ b/plugin/config.json
@@ -3,6 +3,5 @@
     "apiLogPath": null,
     "webBindAddress": "127.0.0.1",
     "webBindPort": 8765,
-    "webCorsOrigin": "http://localhost",
     "webCorsOriginList": ["http://localhost"]
 }
diff --git a/plugin/util.py b/plugin/util.py
index a044814..28c4382 100644
--- a/plugin/util.py
+++ b/plugin/util.py
@@ -74,7 +74,7 @@ def setting(key):
         'webBacklog': 5,
         'webBindAddress': os.getenv('ANKICONNECT_BIND_ADDRESS', '127.0.0.1'),
         'webBindPort': 8765,
-        'webCorsOrigin': os.getenv('ANKICONNECT_CORS_ORIGIN', 'http://localhost'),
+        'webCorsOrigin': os.getenv('ANKICONNECT_CORS_ORIGIN', None),
         'webCorsOriginList': ['http://localhost'],
         'webTimeout': 10000,
     }
diff --git a/plugin/web.py b/plugin/web.py
index 4e435e8..1c1daf6 100644
--- a/plugin/web.py
+++ b/plugin/web.py
@@ -154,14 +154,6 @@ class WebServer:
 
 
     def handlerWrapper(self, req):
-        if len(req.body) == 0:
-            body = 'AnkiConnect v.{}'.format(util.setting('apiVersion')).encode('utf-8')
-        else:
-            try:
-                params = json.loads(req.body.decode('utf-8'))
-                body = json.dumps(self.handler(params)).encode('utf-8')
-            except ValueError:
-                body = json.dumps(None).encode('utf-8')
 
         # handle multiple cors origins by checking the 'origin'-header against the allowed origin list from the config
         webCorsOriginList = util.setting('webCorsOriginList')
@@ -171,25 +163,54 @@ class WebServer:
         if webCorsOrigin:
             webCorsOriginList.append(webCorsOrigin)
 
+        allowed = False
         corsOrigin = 'http://localhost'
         allowAllCors = '*' in webCorsOriginList  # allow CORS for all domains
-        if len(webCorsOriginList) == 1 and not allowAllCors:
-            corsOrigin = webCorsOriginList[0]
+        
+        if allowAllCors:
+            corsOrigin = '*'
+            allowed = True
         elif b'origin' in req.headers:
             originStr = req.headers[b'origin'].decode()
-            if originStr in webCorsOriginList or allowAllCors:
+            if originStr in webCorsOriginList :
                 corsOrigin = originStr
-
-        headers = [
-            ['HTTP/1.1 200 OK', None],
-            ['Content-Type', 'text/json'],
-            ['Access-Control-Allow-Origin', corsOrigin],
-            ['Access-Control-Allow-Headers', '*'],
-            ['Content-Length', str(len(body))]
-        ]
+                allowed = True
+            elif 'http://localhost' in webCorsOriginList and ( 
+            originStr == 'http://127.0.0.1' or originStr == 'https://127.0.0.1' or # allow 127.0.0.1 if localhost allowed
+            originStr.startswith('http://127.0.0.1:') or originStr.startswith('http://127.0.0.1:') or
+            originStr.startswith('chrome-extension://') or originStr.startswith('moz-extension://') ) : # allow chrome and firefox extension if localhost allowed
+                corsOrigin = originStr
+                allowed = True
+        else:
+            allowed = True
 
         resp = bytes()
 
+        if allowed :
+            if len(req.body) == 0:
+                body = 'AnkiConnect v.{}'.format(util.setting('apiVersion')).encode('utf-8')
+            else:
+                try:
+                    params = json.loads(req.body.decode('utf-8'))
+                    body = json.dumps(self.handler(params)).encode('utf-8')
+                except ValueError:
+                    body = json.dumps(None).encode('utf-8')    
+                    
+            headers = [
+                ['HTTP/1.1 200 OK', None],
+                ['Content-Type', 'text/json'],
+                ['Access-Control-Allow-Origin', corsOrigin],
+                ['Access-Control-Allow-Headers', '*'],
+                ['Content-Length', str(len(body))]
+            ]
+        else :
+            headers = [
+                ['HTTP/1.1 403 Forbidden', None],
+                ['Access-Control-Allow-Origin', corsOrigin],
+                ['Access-Control-Allow-Headers', '*']
+            ]
+            body = ''.encode('utf-8');
+
         for key, value in headers:
             if value is None:
                 resp += '{}\r\n'.format(key).encode('utf-8')