Skip to content

Commit

Permalink
Provide custom 404 view that doesn't echo path (#800)
Browse files Browse the repository at this point in the history
We get some bogus bug bounty submissions that talk about HTML injection
on our 404 Not Found pages, which are default Pyramid views which echo
out the `path` of the Request that isn't found. So you can put some
gibberish in there, but it looks real jank and would not be a valid
social engineering attack, but it should be simple for us to just
default this to be quieter so we don't receive these reports.

Ref: https://docs.pylonsproject.org/projects/pyramid/en/latest/narr/hooks.html
Example: https://ads-api.reddit.com/this_is_a_test_where_i_could_spoof_whatever_i_guess

- [x] CI tests (if present) are passing
- [x] Adheres to code style for repo
- [x] Contributor License Agreement (CLA) completed if not a Reddit employee

Co-authored-by: David King <ketralnis@reddit.com>
  • Loading branch information
sp3nx0r and ketralnis committed Aug 7, 2023
1 parent fa188fe commit 1142489
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 0 deletions.
5 changes: 5 additions & 0 deletions baseplate/frameworks/pyramid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ def includeme(self, config: Configurator) -> None:
config.set_request_factory(RequestFactory(self.baseplate))
config.add_subscriber(self._on_new_request, pyramid.events.ContextFound)
config.add_subscriber(self._on_application_created, pyramid.events.ApplicationCreated)
config.add_notfound_view(notfound_override)

# Position of the tween is important. We need it to cover all code
# that can written in the app. This means that it should be above
Expand Down Expand Up @@ -444,3 +445,7 @@ def get_is_healthy_probe(request: Request) -> int:
code,
)
return IsHealthyProbe.READINESS


def notfound_override(_request: Request) -> Response:
return Response("Not Found", status="404 Not Found")
8 changes: 8 additions & 0 deletions tests/integration/pyramid_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,14 @@ def test_not_found(self):
self.assertFalse(self.observer.on_server_span_created.called)
self.assertFalse(self.context_init_event_subscriber.called)

def test_not_found_echo_path(self):
# confirm that issue #800 isn't reintroduced. This is an issue where we
# echo the path to the 404 in the response which probably isn't a
# problem, but does show up in automated vuln scans which can cause some
# extra work hunting down false positives
resp = self.test_app.get("/doesnt_exist", status=404)
self.assertNotIn(b"doesnt_exist", resp.body)

def test_exception_caught(self):
with self.assertRaises(TestException):
self.test_app.get("/example?error")
Expand Down

0 comments on commit 1142489

Please sign in to comment.