search

matrix-org / pantalaimon

E2EE aware proxy daemon for matrix clients.
Apache License 2.0
288 stars 41 forks source link

Sync body decryption parsing is not robust enough #101

Open begincalendar opened 3 years ago

begincalendar commented 3 years ago

Describe the bug Started getting this stacktrace in my log file:

Error handling request                                                          
Traceback (most recent call last):                                              
  File "/usr/local/lib/python3.8/dist-packages/aiohttp/web_protocol.py", line 422, in _handle_request
    resp = await self._request_handler(request)                                 
  File "/usr/local/lib/python3.8/dist-packages/aiohttp/web_app.py", line 499, in _handle
    resp = await handler(request)                                               
  File "/usr/local/lib/python3.8/dist-packages/pantalaimon/daemon.py", line 773, in sync
    json_response = await self.decrypt_body(client, json_response)              
  File "/usr/local/lib/python3.8/dist-packages/pantalaimon/daemon.py", line 732, in decrypt_body
    return await asyncio.wait_for(                                              
  File "/usr/lib/python3.8/asyncio/tasks.py", line 494, in wait_for             
    return fut.result()                                                         
  File "/usr/local/lib/python3.8/dist-packages/pantalaimon/daemon.py", line 725, in decrypt_loop
    return decryption_method(body, ignore_failures=False)                       
  File "/usr/local/lib/python3.8/dist-packages/pantalaimon/client.py", line 909, in decrypt_sync_body
    for room_id, room_dict in body["rooms"]["join"].items():                    
KeyError: 'rooms'

I added a log.error() statement to print the contents of body and got:

{'next_batch': 'xxx_xxx_xxx_xxx_xxx_xxx_xxx_xxx_xxx', 'device_one_time_keys_count': {'signed_curve25519': 50}}

It appears as though the parsing of body assumes that the dict keys will always be present.

I replaced the lookups with .get() calls and that seemed to fix the issue.

To Reproduce Unfortunately I don't have a set of steps, this just started cropping up.

Expected behavior More robust dict parsing.

Additional context I don't know if this is related to this issue that was occurring at the same time: matrix-org/matrix-python-sdk#320

poljar commented 3 years ago

The linked issue isn't related, we're not using matrix-python-sdk and the traceback points to one of our own files.

https://github.com/matrix-org/pantalaimon/pull/100 fixes this.

begincalendar commented 3 years ago

I wasn't sure if it was a coincidence that they both were occurring simultaneously or if there was a connection, so I just linked them in case.