ApiUpload needs sanity check on chunk size
Closed, ResolvedPublic

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:

  • 1.26 - same as master ()
  • 1.25 -
  • 1.24 -
  • 1.23 -

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

RobinHood70 updated the task description. (Show Details)
RobinHood70 raised the priority of this task from to Low.
RobinHood70 added a project: MediaWiki-API.
RobinHood70 added a subscriber: RobinHood70.
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: Security. · View Herald Transcript
RobinHood70 updated the task description. (Show Details)Mar 1 2015, 7:08 PM
Anomie added a subscriber: Anomie.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

Anomie moved this task from Unsorted to Needs Review on the MediaWiki-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.

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.

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: Restricted Task.Mar 19 2015, 9:28 PM
csteipp removed a parent task: Restricted Task.

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

when we do the actual release.

csteipp closed this task as Resolved.

Deployed yesterday

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

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 added a subscriber: demon.Oct 15 2015, 7:57 PM

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

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

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

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

csteipp updated the task description. (Show Details)Oct 16 2015, 4:23 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.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

csteipp updated the task description. (Show Details)Nov 3 2015, 9:08 PM