Skip to content

Commit

Permalink
Be stricter about JSON that is accepted by Sydent (#337)
Browse files Browse the repository at this point in the history
This disables the JSON extensions which Python supports by default (parsing of
`Infinity` / `-Infinity` and `NaN`). These shouldn't be accepted since they're
not technically valid JSON and other languages won't be able to interpret it
properly.

This isn't expected to have any observable effects, since no
correctly-operating endpoint should be returning this invalid JSON anyway.
  • Loading branch information
richvdh authored Apr 6, 2021
1 parent d078590 commit 2e6b00a
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 15 deletions.
1 change: 1 addition & 0 deletions changelog.d/337.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug where invalid JSON would be accepted over the HTTP interfaces.
3 changes: 2 additions & 1 deletion sydent/http/httpclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from sydent.http.matrixfederationagent import MatrixFederationAgent

from sydent.http.federation_tls_options import ClientTLSOptionsFactory
from sydent.util import json_decoder

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -52,7 +53,7 @@ def get_json(self, uri):
body = yield readBody(response)
try:
# json.loads doesn't allow bytes in Python 3.5
json_body = json.loads(body.decode("UTF-8"))
json_body = json_decoder.decode(body.decode("UTF-8"))
except Exception as e:
logger.exception("Error parsing JSON from %s", uri)
raise
Expand Down
6 changes: 3 additions & 3 deletions sydent/http/matrixfederationagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
# limitations under the License.
from __future__ import absolute_import

import json
import logging
import random
import time
Expand All @@ -32,6 +31,7 @@
from twisted.web.iweb import IAgent

from sydent.http.srvresolver import SrvResolver, pick_server_from_list
from sydent.util import json_decoder
from sydent.util.ttlcache import TTLCache

# period to cache .well-known results for by default
Expand Down Expand Up @@ -320,7 +320,7 @@ def _do_get_well_known(self, server_name):
if response.code != 200:
raise Exception("Non-200 response %s" % (response.code, ))

parsed_body = json.loads(body.decode('utf-8'))
parsed_body = json_decoder.decode(body.decode('utf-8'))
logger.info("Response from .well-known: %s", parsed_body)
if not isinstance(parsed_body, dict):
raise Exception("not a dict")
Expand Down Expand Up @@ -439,4 +439,4 @@ class _RoutingResult(object):
The port we should route the TCP connection to (the target of the SRV record, or
the port from the URL/.well-known, or 8448)
:type: int
"""
"""
3 changes: 2 additions & 1 deletion sydent/http/servlets/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from twisted.internet import defer
from twisted.web import server

from sydent.util import json_decoder

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -76,7 +77,7 @@ def get_args(request, args, required=True):
):
try:
# json.loads doesn't allow bytes in Python 3.5
request_args = json.loads(request.content.read().decode("UTF-8"))
request_args = json_decoder.decode(request.content.read().decode("UTF-8"))
except ValueError:
raise MatrixRestError(400, 'M_BAD_JSON', 'Malformed JSON')

Expand Down
7 changes: 3 additions & 4 deletions sydent/http/servlets/lookupservlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
from sydent.db.threepid_associations import GlobalAssociationStore

import logging
import json
import signedjson.sign

from sydent.http.servlets import get_args, jsonwrap, send_cors, MatrixRestError
from sydent.http.auth import authIfV2

from sydent.util import json_decoder

logger = logging.getLogger(__name__)

Expand All @@ -42,7 +41,7 @@ def render_GET(self, request):
Look up an individual threepid.
** DEPRECATED **
Params: 'medium': the medium of the threepid
'address': the address of the threepid
Returns: A signed association if the threepid has a corresponding mxid, otherwise the empty object.
Expand All @@ -63,7 +62,7 @@ def render_GET(self, request):
if not sgassoc:
return {}

sgassoc = json.loads(sgassoc)
sgassoc = json_decoder.decode(sgassoc)
if not self.sydent.server_name in sgassoc['signatures']:
# We have not yet worked out what the proper trust model should be.
#
Expand Down
3 changes: 2 additions & 1 deletion sydent/http/servlets/replication.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
from twisted.web.resource import Resource
from sydent.http.servlets import jsonwrap, MatrixRestError
from sydent.threepid import threePidAssocFromDict
from sydent.util import json_decoder

from sydent.util.hash import sha256_and_url_safe_base64

Expand Down Expand Up @@ -60,7 +61,7 @@ def render_POST(self, request):

try:
# json.loads doesn't allow bytes in Python 3.5
inJson = json.loads(request.content.read().decode("UTF-8"))
inJson = json_decoder.decode(request.content.read().decode("UTF-8"))
except ValueError:
logger.warn("Peer %s made push connection with malformed JSON", peer.servername)
raise MatrixRestError(400, 'M_BAD_JSON', 'Malformed JSON')
Expand Down
7 changes: 4 additions & 3 deletions sydent/http/servlets/threepidunbindservlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

from sydent.http.servlets import dict_to_json_bytes
from sydent.db.valsession import ThreePidValSessionStore
from sydent.util import json_decoder
from sydent.util.stringutils import is_valid_client_secret
from sydent.validators import (
IncorrectClientSecretException,
Expand Down Expand Up @@ -51,7 +52,7 @@ def _async_render_POST(self, request):
try:
try:
# json.loads doesn't allow bytes in Python 3.5
body = json.loads(request.content.read().decode("UTF-8"))
body = json_decoder.decode(request.content.read().decode("UTF-8"))
except ValueError:
request.setResponseCode(400)
request.write(dict_to_json_bytes({'errcode': 'M_BAD_JSON', 'error': 'Malformed JSON'}))
Expand Down Expand Up @@ -81,7 +82,7 @@ def _async_render_POST(self, request):
# and "client_secret" fields, they are trying to prove that they
# were the original author of the bind. We then check that what
# they supply matches and if it does, allow the unbind.
#
#
# However if these fields are not supplied, we instead check
# whether the request originated from a homeserver, and if so the
# same homeserver that originally created the bind. We do this by
Expand Down Expand Up @@ -121,7 +122,7 @@ def _async_render_POST(self, request):
'error': "This validation session has not yet been completed"
}))
return

if s.medium != threepid['medium'] or s.address != threepid['address']:
request.setResponseCode(403)
request.write(dict_to_json_bytes({
Expand Down
3 changes: 2 additions & 1 deletion sydent/replication/peer.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from sydent.db.hashing_metadata import HashingMetadataStore
from sydent.threepid import threePidAssocFromDict
from sydent.config import ConfigError
from sydent.util import json_decoder
from sydent.util.hash import sha256_and_url_safe_base64
from unpaddedbase64 import decode_base64

Expand Down Expand Up @@ -241,7 +242,7 @@ def _failedPushBodyRead(self, body, updateDeferred):
:param updateDeferred: The deferred to call the error callback of.
:type updateDeferred: twisted.internet.defer.Deferred
"""
errObj = json.loads(body.decode("utf8"))
errObj = json_decoder.decode(body.decode("utf8"))
e = RemotePeerError()
e.errorDict = errObj
updateDeferred.errback(e)
Expand Down
11 changes: 10 additions & 1 deletion sydent/util/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import json
import time


def time_msec():
"""
Get the current time in milliseconds.
Expand All @@ -25,3 +25,12 @@ def time_msec():
:rtype: int
"""
return int(time.time() * 1000)


def _reject_invalid_json(val):
"""Do not allow Infinity, -Infinity, or NaN values in JSON."""
raise ValueError("Invalid JSON value: '%s'" % val)


# a custom JSON decoder which will reject Python extensions to JSON.
json_decoder = json.JSONDecoder(parse_constant=_reject_invalid_json)

0 comments on commit 2e6b00a

Please sign in to comment.