Skip to content

🔒️(backend) fix two WOPI security bypasses - #795

Merged
kernicPanel merged 2 commits into
mainfrom
fix/wopi-security-bypasses
Aug 7, 2026
Merged

🔒️(backend) fix two WOPI security bypasses#795
kernicPanel merged 2 commits into
mainfrom
fix/wopi-security-bypasses

Conversation

@kernicPanel

Copy link
Copy Markdown
Collaborator

Purpose

Two WOPI endpoints let a standard user store a file that neither the
allowed extension list nor the antivirus ever checked.

RENAME_FILE used the requested name as decoded, without validating the
resulting extension. PutFile wrote content straight to object storage
without triggering malware detection, which only ran on the upload
endpoint.

Proposal

  • Reject a rename whose target contains a path separator or lands on a
    disallowed extension, and compare extensions case insensitively
  • Run malware detection on the content written by PutFile

@kernicPanel
kernicPanel requested review from NathanVss and lunika August 3, 2026 15:00
invalid_filename_error = "No filename provided"
else:
# Convert it to utf-7 to avoid issues with special characters
new_filename = new_filename.encode("ascii").decode("utf-7")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why utf-7, why not utf-8 as utf-7 is considered obsolete?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

utf-7 is part of wopi specs…

Comment thread src/backend/wopi/viewsets.py Outdated
# non-READY files in WOPI.
item.save(update_fields=["size", "updated_at"])

malware_detection.analyse_file(item.file_key, item_id=item.id)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This approach leave a window during which a dangerous file is in READY state.

That being said, to avoid degrading live edition I understand we have no better choice for now.
The fine grade solution is indeed only allowing wopi editors to access the wopi API via this PR: #416

@kernicPanel
kernicPanel force-pushed the fix/wopi-security-bypasses branch from 97af154 to a4ba208 Compare August 7, 2026 08:33
The requested name was decoded from UTF-7 and used as is, so a path
separator could reach the storage key and leave the item without an
extension. A second rename then granted it any extension, bypassing the
allowed list that only the upload path enforced.

Extensions are now compared case insensitively on both sides, so entries
declared with capitals are no longer dead.
PutFile wrote the content straight to object storage, while malware
detection only ran on the upload endpoint. Editing a document was
therefore a way to store content that was never analysed.

The item stays READY during the analysis: a collaborator cannot open a
file that is not READY, so flipping the state would eject everyone from
the document on every save.
@kernicPanel
kernicPanel force-pushed the fix/wopi-security-bypasses branch from 8487176 to 9ec7288 Compare August 7, 2026 10:39
@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
B Security Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@kernicPanel
kernicPanel merged commit 9ec7288 into main Aug 7, 2026
32 of 33 checks passed
@kernicPanel
kernicPanel deleted the fix/wopi-security-bypasses branch August 7, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants