Page MenuHomePhabricator
Create Task
Maniphest T91205

ApiUpload needs sanity check on chunk size
Closed, ResolvedPublic
Actions

Assigned To
Anomie
Authored By
RobinHood70
Mar 1 2015, 6:55 PM
Tags
Referenced Files
F2727436: T91205-REL1_23v2.patch
Oct 16 2015, 4:12 PM
F2724623: T91205-REL1_23.patch
Oct 15 2015, 8:49 PM
F2724606: T91205-REL1_24.patch
Oct 15 2015, 8:38 PM
F2724580: T91205-REL1_25.patch
Oct 15 2015, 8:23 PM
F2558076: T91205_1.26wmf22.patch
Sep 8 2015, 6:20 PM
F52684: 0001-API-Improve-validation-in-chunked-uploading.patch
Mar 2 2015, 9:59 PM
Subscribers
Aklapper
• csteipp
• demon
Ejegg
gerritbot
Grunny
matmarex
View All 9 Subscribers
Tokens
"Pterodactyl" token, awarded by demon.

Description

There is nothing preventing a badly coded or malicious client from sending a file in chunks of one byte. This could be exploited to flood a server with files and create file management overhead as well. All but the final block should have a minimum chunk size. There should also be a "final chunk" marker of some kind to allow only one final chunk and not several. This avoids a near-identical exploit where the "final" chunk is a single byte, but is kept below an ever-changing filesize.

patch:

T91205_1.26wmf22.patch8 KBDownload

  • 1.26 - same as master (
    T91205_1.26wmf22.patch8 KBDownload
    )
  • 1.25 -
    T91205-REL1_25.patch8 KBDownload
  • 1.24 -
    T91205-REL1_24.patch8 KBDownload
  • 1.23 -
    T91205-REL1_23v2.patch8 KBDownload

affected versions:
type: dos
CVE: CVE-2015-8002

Details

SubjectRepoBranchLines +/-
SECURITY: API: Improve validation in chunked uploadingmediawiki/coremaster+90 -16
SECURITY: API: Improve validation in chunked uploadingmediawiki/coreREL1_26+90 -16
SECURITY: API: Improve validation in chunked uploadingmediawiki/coreREL1_23+89 -17
SECURITY: API: Improve validation in chunked uploadingmediawiki/coreREL1_24+89 -17
SECURITY: API: Improve validation in chunked uploadingmediawiki/coreREL1_25+89 -16
Customize query in gerrit

Related Objects
Search...

Event Timeline

RobinHood70 created this task.Mar 1 2015, 6:55 PM
RobinHood70 raised the priority of this task from to Low.
RobinHood70 updated the task description. (Show Details)
RobinHood70 added a project: MediaWiki-Action-API.
RobinHood70 subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 1 2015, 6:55 PM
RobinHood70 updated the task description. (Show Details)Mar 1 2015, 6:56 PM
RobinHood70 set Security to None.
RobinHood70 changed Security from None to Software security bug.Mar 1 2015, 7:00 PM
Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptMar 1 2015, 7:00 PM
Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript
Restricted Application added a project: acl*security. · View Herald Transcript
RobinHood70 updated the task description. (Show Details)Mar 1 2015, 7:08 PM
Anomie subscribed.Mar 2 2015, 7:22 PM

How does this differ from someone just stashing a lot of 1-byte files? Or uploading only the first chunk for many different filenames?

There should also be a "final chunk" marker of some kind to allow only one final chunk and not several.

A chunk is "final" if the offset + the length of data uploaded == the claimed total file size, all three of which are already supplied. All we'd need to do is save some indicator that this has occurred and throw an error if another chunk (with a larger file size) is supplied later.

Looking at the code, this may already be happening: in async mode a flag is set indicating that chunk assembly is already in progress (although only after the new chunk is accepted), while in sync mode the old filekey is entirely deleted after the chunks are assembled.

Anomie claimed this task.Mar 2 2015, 9:59 PM

0001-API-Improve-validation-in-chunked-uploading.patch10 KBDownload

Anomie added a project: Patch-For-Review.Mar 2 2015, 9:59 PM
Anomie mentioned this in T91203: ApiUpload allows overrun without error.
Anomie moved this task from Unsorted to Needs Review on the MediaWiki-Action-API board.
Anomie added a project: MediaWiki-Core-Team.
Anomie moved this task from Backlog to Needs Review/Feedback on the MediaWiki-Core-Team board.
RobinHood70 added a comment.Mar 2 2015, 10:23 PM

How does this differ from someone just stashing a lot of 1-byte files? Or uploading only the first chunk for many different filenames?

Also good points. My understanding of programming for a web environment is limited, but perhaps there could be some kind of timeout mechanism or background task that checks for these situations, aborting and deleting leftover files after a certain period of time.

I can confirm from my own testing that sending chunks after the stated filesize has been exceeded is allowed, though you do need to send a chunk that doesn't precisely match the filesize (i.e., if the file should only have 1000 bytes left and you send 2000, it will allow you to continue sending indefinitely after that). It looks like your changes will fix that issue, though.

• csteipp subscribed.Mar 9 2015, 11:22 PM
In T91205#1079974, @Anomie wrote:

0001-API-Improve-validation-in-chunked-uploading.patch10 KBDownload

Looks like this will prevent the issue, and seems to work fine in my testing. I'll get this deployed soon.

• csteipp added a parent task: T91850: No rate limits on uploading files.Mar 9 2015, 11:25 PM
• csteipp added a parent task: Restricted Task.Mar 19 2015, 9:28 PM
Anomie edited projects, added Reading-Infrastructure-Team-Old (Don't use); removed MediaWiki-Core-Team.Mar 30 2015, 7:44 PM
Anomie moved this task from Backlog to Done on the Reading-Infrastructure-Team-Old (Don't use) board.
• csteipp added a parent task: T108064: MediaWiki Security release 1.25.3.Aug 5 2015, 5:17 PM
• csteipp removed a parent task: Restricted Task.
• csteipp added projects: Security-Core, Vuln-DoS.Aug 20 2015, 9:35 PM
• csteipp added a comment.Sep 8 2015, 6:20 PM

T91205_1.26wmf22.patch8 KBDownload

Rebased, removed release notes section (to prevent further merge conflicts), and reduced the minimum block size to 1KB (in the interest of not causing problems on the site with a security patch, we can raise it to 1MB if we ever see it being abused).

We'll add in the release notes from

0001-API-Improve-validation-in-chunked-uploading.patch10 KBDownload
when we do the actual release.

• csteipp closed this task as Resolved.Sep 9 2015, 10:13 PM
• csteipp mentioned this in T91850: No rate limits on uploading files.
• csteipp mentioned this in T108616: Local path disclosure when using ImageMagick as a scaler.

Deployed yesterday

(2015-09-08) 21:02 csteipp: deployed patches for T108616 T91850 T91205 to wmf21 & 22

matmarex subscribed.Sep 12 2015, 9:21 PM

This patch makes some mostly unrelated checks stricter, which resulted in T111908. The 'offset' parameter should probably default to 0 to replicate previous behavior.

matmarex reopened this task as Open.Sep 12 2015, 9:21 PM

According to T112405, the check is not applied to the first chunk?

Anomie closed this task as Resolved.Sep 14 2015, 5:09 PM

T112405 was a misunderstanding of how chunked uploading and warnings interact.

• demon subscribed.Oct 15 2015, 7:57 PM

T91205_1.26wmf22.patch8 KBDownload
only applies cleanly to master and REL1_26, will work up patches for 23, 24 and 25.

• demon added a comment.Oct 15 2015, 8:23 PM

T91205-REL1_25.patch8 KBDownload

• demon added a comment.Oct 15 2015, 8:38 PM

T91205-REL1_24.patch8 KBDownload

• demon added a comment.Oct 15 2015, 8:49 PM

T91205-REL1_23.patch7 KBDownload

• demon awarded a token.Oct 15 2015, 8:49 PM
• csteipp added a subscriber: Grunny.Oct 16 2015, 3:03 PM
• csteipp added a subscriber: ProgramCeltic.Oct 16 2015, 4:10 PM
• demon added a comment.Oct 16 2015, 4:12 PM

T91205-REL1_23v2.patch8 KBDownload

• csteipp updated the task description. (Show Details)Oct 16 2015, 4:23 PM
• csteipp added a subscriber: Ejegg.Oct 16 2015, 4:38 PM
• demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 16 2015, 6:12 PM
• demon changed the edit policy from "Custom Policy" to "All Users".
Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptOct 16 2015, 6:12 PM
Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript
• demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 16 2015, 6:13 PM
• demon changed the edit policy from "Custom Policy" to "All Users".
• demon changed Security from Software security bug to None.
gerritbot subscribed.Oct 16 2015, 6:19 PM

Change 246869 had a related patch set uploaded (by Chad):
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246869

gerritbot added a comment.Oct 16 2015, 6:19 PM

Change 246874 had a related patch set uploaded (by Chad):
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246874

gerritbot added a comment.Oct 16 2015, 6:20 PM

Change 246880 had a related patch set uploaded (by Chad):
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246880

gerritbot added a comment.Oct 16 2015, 6:24 PM

Change 246885 had a related patch set uploaded (by Chad):
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246885

gerritbot added a comment.Oct 16 2015, 7:03 PM

Change 246869 merged by jenkins-bot:
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246869

gerritbot added a comment.Oct 16 2015, 7:49 PM

Change 246880 merged by jenkins-bot:
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246880

gerritbot added a comment.Oct 16 2015, 7:51 PM

Change 246874 merged by jenkins-bot:
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246874

ReleaseTaggerBot added projects: MW-1.25-release, MW-1.24-release, MW-1.23-release.Oct 16 2015, 8:00 PM
gerritbot added a comment.Oct 16 2015, 8:56 PM

Change 246974 had a related patch set uploaded (by Chad):
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246974

gerritbot added a comment.Oct 16 2015, 9:19 PM

Change 246974 merged by jenkins-bot:
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246974

gerritbot added a comment.Oct 16 2015, 9:45 PM

Change 246885 merged by jenkins-bot:
SECURITY: API: Improve validation in chunked uploading

https://gerrit.wikimedia.org/r/246885

ReleaseTaggerBot added projects: MW-1.27-release-notes, MW-1.27-release (WMF-deploy-2015-10-27_(1.27.0-wmf.4)), MW-1.26-release.Oct 16 2015, 10:00 PM
• csteipp updated the task description. (Show Details)Nov 3 2015, 9:08 PM
• chasemp added a project: Security.Feb 10 2020, 11:00 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:18 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:43 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL