ApiUpload allows overrun without error
Closed, ResolvedPublic

Description

In chunked mode, ApiUpload allows a client to send data past the stated filesize. If the filesize parameter is kept suitably small, I believe a badly coded or malicious client could conceivably upload indefinitely.


patches:

  • Patches for both this issue and T91205 are attached to T91205.

affected versions:
type: DoS
CVE: CVE-2015-8001

RobinHood70 added a project: MediaWiki-API.
RobinHood70 added a subscriber: RobinHood70.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 1 2015, 6:44 PM
Ciencia_Al_Poder set Security to Software security bug.Mar 1 2015, 6:58 PM
Ciencia_Al_Poder added a subscriber: Ciencia_Al_Poder.
Restricted Application changed the visibility from "Public (No Login Required)" to "Custom Policy". · View Herald TranscriptMar 1 2015, 6:58 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

I've preemptively activated the security flag of this bug just in case this could be a real security issue, as the description suggests.

Anomie added a subscriber: Anomie.Mar 2 2015, 7:28 PM

The filesize can never be bigger than $wgMaxUploadSize, so they can't keep increasing it indefinitely.

But it does seem the API could use some additional input validation, as I don't see anything that checks whether the supplied offset plus the length of the chunk is greater than the claimed filesize, and doing so would allow uploading until overflow on the offset parameter.

Anomie claimed this task.EditedMar 2 2015, 10:00 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.

The filesize can never be bigger than $wgMaxUploadSize, so they can't keep increasing it indefinitely.

Just to follow-up here for those not following T91205, they don't actually need to increase filesize currently. As long as the so-called final chunk exceeds the filesize rather than equalling it, the upload can continue. You can set filesize to 1000, upload 2000 bytes in a chunk, then continue uploading data for as long as you wish, as long as you continue to claim that the filesize is 1000 bytes. The patch you've submitted at T91205 will fix that issue.

ksmith added a subscriber: ksmith.Mar 24 2015, 10:49 PM

Anomie: Can you (or someone) post a comment here showing the current status of this?

I reviewed the patch (I think it's the same one?) on T91205. I'll deploy it to the cluster in the near future. T91205 / this issue will get patched in 1.24.3, probably next month.

I reviewed the patch (I think it's the same one?) on T91205.

Yes, same patch. I suppose I could have split them up, but the two would probably merge-conflict or one would depend on the other.

Anomie closed this task as "Resolved".Sep 10 2015, 2:10 PM

Per T91205#1622745, since it's the same patch for both tasks.

demon awarded a token.Oct 15 2015, 8:58 PM
csteipp added a subscriber: Grunny.Oct 16 2015, 3:05 PM
csteipp edited the task description. (Show Details)Oct 16 2015, 4:32 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:11 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:11 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:11 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 edited the task description. (Show Details)Nov 3 2015, 9:07 PM
ksmith removed a subscriber: ksmith.Nov 3 2015, 9:50 PM