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
This commit is contained in:
DegrangeM 2021-05-06 07:31:11 +02:00 committed by GitHub
parent 929716c7b1
commit 9472ac4401
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 21 deletions

View File

@ -3,6 +3,5 @@
"apiLogPath": null, "apiLogPath": null,
"webBindAddress": "127.0.0.1", "webBindAddress": "127.0.0.1",
"webBindPort": 8765, "webBindPort": 8765,
"webCorsOrigin": "http://localhost",
"webCorsOriginList": ["http://localhost"] "webCorsOriginList": ["http://localhost"]
} }

View File

@ -74,7 +74,7 @@ def setting(key):
'webBacklog': 5, 'webBacklog': 5,
'webBindAddress': os.getenv('ANKICONNECT_BIND_ADDRESS', '127.0.0.1'), 'webBindAddress': os.getenv('ANKICONNECT_BIND_ADDRESS', '127.0.0.1'),
'webBindPort': 8765, 'webBindPort': 8765,
'webCorsOrigin': os.getenv('ANKICONNECT_CORS_ORIGIN', 'http://localhost'), 'webCorsOrigin': os.getenv('ANKICONNECT_CORS_ORIGIN', None),
'webCorsOriginList': ['http://localhost'], 'webCorsOriginList': ['http://localhost'],
'webTimeout': 10000, 'webTimeout': 10000,
} }

View File

@ -154,14 +154,6 @@ class WebServer:
def handlerWrapper(self, req): 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 # handle multiple cors origins by checking the 'origin'-header against the allowed origin list from the config
webCorsOriginList = util.setting('webCorsOriginList') webCorsOriginList = util.setting('webCorsOriginList')
@ -171,25 +163,54 @@ class WebServer:
if webCorsOrigin: if webCorsOrigin:
webCorsOriginList.append(webCorsOrigin) webCorsOriginList.append(webCorsOrigin)
allowed = False
corsOrigin = 'http://localhost' corsOrigin = 'http://localhost'
allowAllCors = '*' in webCorsOriginList # allow CORS for all domains 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: elif b'origin' in req.headers:
originStr = req.headers[b'origin'].decode() originStr = req.headers[b'origin'].decode()
if originStr in webCorsOriginList or allowAllCors: if originStr in webCorsOriginList :
corsOrigin = originStr corsOrigin = originStr
allowed = True
headers = [ elif 'http://localhost' in webCorsOriginList and (
['HTTP/1.1 200 OK', None], originStr == 'http://127.0.0.1' or originStr == 'https://127.0.0.1' or # allow 127.0.0.1 if localhost allowed
['Content-Type', 'text/json'], originStr.startswith('http://127.0.0.1:') or originStr.startswith('http://127.0.0.1:') or
['Access-Control-Allow-Origin', corsOrigin], originStr.startswith('chrome-extension://') or originStr.startswith('moz-extension://') ) : # allow chrome and firefox extension if localhost allowed
['Access-Control-Allow-Headers', '*'], corsOrigin = originStr
['Content-Length', str(len(body))] allowed = True
] else:
allowed = True
resp = bytes() 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: for key, value in headers:
if value is None: if value is None:
resp += '{}\r\n'.format(key).encode('utf-8') resp += '{}\r\n'.format(key).encode('utf-8')