Chad has uploaded a new change for review.
https://gerrit.wikimedia.org/r/246880
Change subject: SECURITY: API: Improve validation in chunked uploading
......................................................................
SECURITY: API: Improve validation in chunked uploading
This fixes a few shortcomings in the chunked uploader:
* Raises an error if offset + chunksize > filesize.
* Enforces a minimum chunk size for non-final chunks.
* Refuses additional chunks after seeing a final chunk.
* Status of a chunked upload in progress is now available with
'checkstatus'.
Bug: T91203
Bug: T91205
Change-Id: I2262db1bc8460616b069c564475d2e4148001768
---
M includes/DefaultSettings.php
M includes/GlobalFunctions.php
M includes/Setup.php
M includes/api/ApiQuerySiteinfo.php
M includes/api/ApiUpload.php
5 files changed, 85 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/80/246880/1
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index cededac..4e5c8fd 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -670,6 +670,14 @@
*/
$wgMaxUploadSize = 1024 * 1024 * 100; # 100MB
+ /**
+ * Minimum upload chunk size, in bytes. When using chunked upload, non-final
+ * chunks smaller than this will be rejected. May be reduced based on the
+ * 'upload_max_filesize' or 'post_max_size' PHP settings.
+ * @since 1.26
+ */
+$wgMinUploadChunkSize = 1024; # 1KB
+
/**
* Point the upload navigation link to an external URL
* Useful if you want to use a shared repository by default
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 8941b97..ef6a53f 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -3942,12 +3942,13 @@
* Converts shorthand byte notation to integer form
*
* @param string $string
+ * @param int $default Returned if $string is empty
* @return int
*/
-function wfShorthandToInteger( $string = '' ) {
+function wfShorthandToInteger( $string = '', $default = -1 ) {
$string = trim( $string );
if ( $string === '' ) {
- return -1;
+ return $default;
}
$last = $string[strlen( $string ) - 1];
$val = intval( $string );
diff --git a/includes/Setup.php b/includes/Setup.php
index b20ce14..0ce1733 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -295,6 +295,15 @@
$wgResourceLoaderMaxQueryLength = $maxValueLength > 0 ? $maxValueLength
: -1;
}
+// Ensure the minimum chunk size is less than PHP upload limits or the maximum
+// upload size.
+$wgMinUploadChunkSize = min(
+ $wgMinUploadChunkSize,
+ $wgMaxUploadSize,
+ wfShorthandToInteger( ini_get( 'upload_max_filesize' ), 1e100 ),
+ wfShorthandToInteger( ini_get( 'post_max_size' ), 1e100) - 1024 # Leave
room for other parameters
+);
+
/**
* Definitions of the NS_ constants are in Defines.php
* @private
diff --git a/includes/api/ApiQuerySiteinfo.php
b/includes/api/ApiQuerySiteinfo.php
index b0e9bd2..97afe7e 100644
--- a/includes/api/ApiQuerySiteinfo.php
+++ b/includes/api/ApiQuerySiteinfo.php
@@ -247,6 +247,7 @@
}
$data['maxuploadsize'] = UploadBase::getMaxUploadSize();
+ $data['minuploadchunksize'] = (int)$this->getConfig()->get(
'MinUploadChunkSize' );
$data['thumblimits'] = $GLOBALS['wgThumbLimits'];
$this->getResult()->setIndexedTagName( $data['thumblimits'],
'limit' );
diff --git a/includes/api/ApiUpload.php b/includes/api/ApiUpload.php
index 354bd0d..28c8d4a 100644
--- a/includes/api/ApiUpload.php
+++ b/includes/api/ApiUpload.php
@@ -82,7 +82,7 @@
// Check if the uploaded file is sane
if ( $this->mParams['chunk'] ) {
- $maxSize = $this->mUpload->getMaxUploadSize();
+ $maxSize = UploadBase::getMaxUploadSize();
if ( $this->mParams['filesize'] > $maxSize ) {
$this->dieUsage( 'The file you submitted was
too large', 'file-too-large' );
}
@@ -202,13 +202,30 @@
private function getChunkResult( $warnings ) {
$result = array();
- $result['result'] = 'Continue';
if ( $warnings && count( $warnings ) > 0 ) {
$result['warnings'] = $warnings;
}
+
$request = $this->getMain()->getRequest();
$chunkPath = $request->getFileTempname( 'chunk' );
$chunkSize = $request->getUpload( 'chunk' )->getSize();
+ $totalSoFar = $this->mParams['offset'] + $chunkSize;
+ $minChunkSize = $this->getConfig()->get( 'MinUploadChunkSize' );
+
+ // Sanity check sizing
+ if ( $totalSoFar > $this->mParams['filesize'] ) {
+ $this->dieUsage(
+ 'Offset plus current chunk is greater than
claimed file size', 'invalid-chunk'
+ );
+ }
+
+ // Enforce minimum chunk size
+ if ( $totalSoFar != $this->mParams['filesize'] && $chunkSize <
$minChunkSize ) {
+ $this->dieUsage(
+ "Minimum chunk size is $minChunkSize bytes for
non-final chunks", 'chunk-too-small'
+ );
+ }
+
if ( $this->mParams['offset'] == 0 ) {
try {
$filekey = $this->performStash();
@@ -218,23 +235,29 @@
}
} else {
$filekey = $this->mParams['filekey'];
+
+ // Don't allow further uploads to an already-completed
session
+ $progress = UploadBase::getSessionStatus(
$this->getUser(), $filekey );
+ if ( !$progress ) {
+ // Probably can't get here, but check anyway
just in case
+ $this->dieUsage( 'No chunked upload session
with this key', 'stashfailed' );
+ } elseif ( $progress['result'] !== 'Continue' ||
$progress['stage'] !== 'uploading' ) {
+ $this->dieUsage(
+ 'Chunked upload is already completed,
check status for details', 'stashfailed'
+ );
+ }
+
/** @var $status Status */
$status = $this->mUpload->addChunk(
$chunkPath, $chunkSize,
$this->mParams['offset'] );
if ( !$status->isGood() ) {
$this->dieUsage( $status->getWikiText(),
'stashfailed' );
-
- return array();
}
}
// Check we added the last chunk:
- if ( $this->mParams['offset'] + $chunkSize ==
$this->mParams['filesize'] ) {
+ if ( $totalSoFar == $this->mParams['filesize'] ) {
if ( $this->mParams['async'] ) {
- $progress = UploadBase::getSessionStatus(
$filekey );
- if ( $progress && $progress['result'] ===
'Poll' ) {
- $this->dieUsage( "Chunk assembly
already in progress.", 'stashfailed' );
- }
UploadBase::setSessionStatus(
$filekey,
array( 'result' => 'Poll',
@@ -258,21 +281,37 @@
} else {
$status = $this->mUpload->concatenateChunks();
if ( !$status->isGood() ) {
+ UploadBase::setSessionStatus(
+ $this->getUser(),
+ $filekey,
+ array( 'result' => 'Failure',
'stage' => 'assembling', 'status' => $status )
+ );
$this->dieUsage(
$status->getWikiText(), 'stashfailed' );
-
- return array();
}
// The fully concatenated file has a new
filekey. So remove
// the old filekey and fetch the new one.
+ UploadBase::setSessionStatus( $this->getUser(),
$filekey, false );
$this->mUpload->stash->removeFile( $filekey );
$filekey =
$this->mUpload->getLocalFile()->getFileKey();
$result['result'] = 'Success';
}
+ } else {
+ UploadBase::setSessionStatus(
+ $this->getUser(),
+ $filekey,
+ array(
+ 'result' => 'Continue',
+ 'stage' => 'uploading',
+ 'offset' => $totalSoFar,
+ 'status' => Status::newGood(),
+ )
+ );
+ $result['result'] = 'Continue';
+ $result['offset'] = $totalSoFar;
}
$result['filekey'] = $filekey;
- $result['offset'] = $this->mParams['offset'] + $chunkSize;
return $result;
}
@@ -381,6 +420,10 @@
// Chunk upload
$this->mUpload = new UploadFromChunks();
if ( isset( $this->mParams['filekey'] ) ) {
+ if ( $this->mParams['offset'] === 0 ) {
+ $this->dieUsage( 'Cannot supply a
filekey when offset is 0', 'badparams' );
+ }
+
// handle new chunk
$this->mUpload->continueChunks(
$this->mParams['filename'],
@@ -735,8 +778,15 @@
),
'stash' => false,
- 'filesize' => null,
- 'offset' => null,
+ 'filesize' => array(
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_MIN => 0,
+ ApiBase::PARAM_MAX =>
UploadBase::getMaxUploadSize(),
+ ),
+ 'offset' => array(
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_MIN => 0,
+ ),
'chunk' => array(
ApiBase::PARAM_TYPE => 'upload',
),
--
To view, visit https://gerrit.wikimedia.org/r/246880
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2262db1bc8460616b069c564475d2e4148001768
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_23
Gerrit-Owner: Chad <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
