Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix recursion error when fetching auth chain over federation #7817

Merged
merged 9 commits into from
Jul 10, 2020
Merged

Fix recursion error when fetching auth chain over federation #7817

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e963a55
Fix recursion error when fetching auth chain
erikjohnston Jul 10, 2020
2b3916d
Newsfile
erikjohnston Jul 10, 2020
cad85d5
Review comments
erikjohnston Jul 10, 2020
7825a59
Fix tests
erikjohnston Jul 10, 2020
c6df9ac
get_events requires a list...
erikjohnston Jul 10, 2020
f6856d8
Make exception autherror in check auth
erikjohnston Jul 10, 2020
f7488b9
Merge branch 'develop' of github.com:matrix-org/synapse into erikj/fi…
erikjohnston Jul 10, 2020
212f9e7
Apply suggestions from code review
erikjohnston Jul 10, 2020
e0c9f59
Use 403 and update comment
erikjohnston Jul 10, 2020
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
5 changes: 3 additions & 2 deletions synapse/event_auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,11 @@ def check(
# sanity-check it
for auth_event in auth_events.values():
if auth_event.room_id != room_id:
raise Exception(
raise AuthError(
500,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a 403?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh oof there is AuthError(500) above. whatever.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yes, probably actually. I was copying it from a previous line, but I don't think that makes sense

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but also: if we actually hit this code and it matters, the comment needs changing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/me awards past me a prize for including sanity checks for security-critical things

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, which comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found it

"During auth for event %s in room %s, found event %s in the state "
"which is in room %s"
% (event.event_id, room_id, auth_event.event_id, auth_event.room_id)
% (event.event_id, room_id, auth_event.event_id, auth_event.room_id),
)

if do_sig_check:
Expand Down