Skip to content

fix: security verified sender overwritten - #9516

Open
pjwerneck wants to merge 2 commits into
devfrom
fix/security-verified-sender-overwritten
Open

pjwerneck wants to merge 2 commits into
devfrom
fix/security-verified-sender-overwritten

Conversation

@pjwerneck

Copy link
Copy Markdown
Collaborator

Summary

The DO read the sender of a proposed file change from the message body, which
the sender writes. The sender is now the peer whose inbox held the
message.

Changes

  • datasite_owner_syncer.py: drop the sender_email = message.sender_email overwrite in pull_and_process_next_proposed_filechange. A body that disagrees with the transport peer is logged and dropped, not processed.
  • datasite_owner_syncer.py: add same_email, a casefolded compare that refuses on None. Email addresses have no case, but _user_in_list matches exactly, so an uppercase claim previously refused an honest write.
  • datasite_owner_syncer.py: build filtered_message from the transport email.
  • gdrive_transport.py / connection_router.py: pass sender_email into owner_remove_proposed_filechange_message_from_inbox. It named — and created — the archive folder from the body, so a spoof placed an attacker-chosen folder in the DO's Drive.

Testing

  • tests/unit/test_sender_binding.py, three cases: a claim of the DO's email writes nothing; a spoofed claim names no archive folder; a claim differing only in case still writes.
  • Each fails against the unpatched code with the expected assertion.
  • The honest path is already covered — stubbing same_email to False fails six existing tests in test_sync_manager.py.
  • Full suite: 432 unit, 251 migrations/rds/enclave. ruff check and format --check clean.

Asana task

https://app.asana.com/1/1185126988600652/project/1216249688888494/task/1218392226542034?focus=true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant