Skip to content

Commit

Permalink
Be stricter about JSON that is accepted by Sygnal (#216)
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.
  • Loading branch information
richvdh authored Apr 13, 2021
1 parent 75ee260 commit 214497a
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 6 deletions.
1 change: 1 addition & 0 deletions changelog.d/216.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.
7 changes: 3 additions & 4 deletions sygnal/gcmpushkin.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import logging
import time
from io import BytesIO
from json import JSONDecodeError

from opentracing import logs, tags
from prometheus_client import Counter, Gauge, Histogram
Expand All @@ -33,7 +32,7 @@
)
from sygnal.helper.context_factory import ClientTLSOptionsFactory
from sygnal.helper.proxy.proxyagent_twisted import ProxyAgent
from sygnal.utils import NotificationLoggerAdapter, twisted_sleep
from sygnal.utils import NotificationLoggerAdapter, json_decoder, twisted_sleep

from .exceptions import PushkinSetupException
from .notifications import ConcurrencyLimitedPushkin
Expand Down Expand Up @@ -251,8 +250,8 @@ async def _request_dispatch(self, n, log, body, headers, pushkeys, span):
return pushkeys, []
elif 200 <= response.code < 300:
try:
resp_object = json.loads(response_text)
except JSONDecodeError:
resp_object = json_decoder.decode(response_text)
except ValueError:
raise NotificationDispatchException("Invalid JSON response from GCM.")
if "results" not in resp_object:
log.error(
Expand Down
4 changes: 2 additions & 2 deletions sygnal/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
from twisted.web.server import NOT_DONE_YET

from sygnal.notifications import NotificationContext
from sygnal.utils import NotificationLoggerAdapter
from sygnal.utils import NotificationLoggerAdapter, json_decoder

from .exceptions import InvalidNotificationException, NotificationDispatchException
from .notifications import Notification
Expand Down Expand Up @@ -133,7 +133,7 @@ def _handle_request(self, request):
log = NotificationLoggerAdapter(logger, {"request_id": request_id})

try:
body = json.loads(request.content.read())
body = json_decoder.decode(request.content.read().decode("utf-8"))
except Exception as exc:
msg = "Expected JSON request body"
log.warning(msg, exc_info=exc)
Expand Down
10 changes: 10 additions & 0 deletions sygnal/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import json
import re
from logging import LoggerAdapter

Expand Down Expand Up @@ -62,3 +63,12 @@ def glob_to_regex(glob):

# \A anchors at start of string, \Z at end of string
return re.compile(r"\A" + res + r"\Z", re.IGNORECASE)


def _reject_invalid_json(val):
"""Do not allow Infinity, -Infinity, or NaN values in JSON."""
raise ValueError(f"Invalid JSON value: {val!r}")


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

0 comments on commit 214497a

Please sign in to comment.