Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be stricter about JSON that is accepted by Sygnal #216

Merged
merged 5 commits into from
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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:
callahad marked this conversation as resolved.
Show resolved Hide resolved
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)