Page MenuHomePhabricator
Authored By
csteipp
Nov 19 2015, 9:30 PM
Size
4 KB
Referenced Files
None
Subscribers
None

T118032b.patch

From 2c4eb0ef6d3dd9513fa17f28777eb66153065754 Mon Sep 17 00:00:00 2001
From: Roan Kattouw <roan.kattouw@gmail.com>
Date: Fri, 6 Nov 2015 12:55:16 -0800
Subject: [PATCH] SECURITY: Work around CURL insanity breaking POST parameters
that start with '@'
CURL has a "feature" where passing array( 'foo' => '@bar' )
in CURLOPT_POSTFIELDS results in the contents of the file named "bar"
being POSTed. This makes it impossible to POST the literal string "@bar",
because array( 'foo' => '%40bar' ) gets double-encoded to foo=%2540bar.
Disable this "feature" by setting CURLOPT_SAFE_UPLOAD to true,
if available. According to the PHP manual, this option became
available in 5.5 and started defaulting to true in 5.6.
However, we support versions as low as 5.3, and this option
doesn't exist at all in 5.6.99-hhvm, which we run in production.
For versions where this option is not available (pre-5.5 versions
and HHVM), serialize POSTFIELDS arrays to strings. This works
around the issue because the '@' "feature" only works
for arrays, not strings, as of PHP 5.2. (We don't support pre-5.2
versions, and I've verified 5.6.99-hhvm behaves this way as well.)
Bug: T118032
Change-Id: I3f996e2eb87c7bd3b94ca9d3cc14a3e12f34f241
---
includes/HttpFunctions.php | 17 ++++++++++++++++-
includes/libs/MultiHttpClient.php | 13 +++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php
index 60196ab..05b97b5 100644
--- a/includes/HttpFunctions.php
+++ b/includes/HttpFunctions.php
@@ -781,7 +781,22 @@ class CurlHttpRequest extends MWHttpRequest {
$this->curlOptions[CURLOPT_HEADER] = true;
} elseif ( $this->method == 'POST' ) {
$this->curlOptions[CURLOPT_POST] = true;
- $this->curlOptions[CURLOPT_POSTFIELDS] = $this->postData;
+ $postData = $this->postData;
+ // Don't interpret POST parameters starting with '@' as file uploads, because this
+ // makes it impossible to POST plain values starting with '@' (and causes security
+ // issues potentially exposing the contents of local files).
+ // The PHP manual says this option was introduced in PHP 5.5 defaults to true in PHP 5.6,
+ // but we support lower versions, and the option doesn't exist in HHVM 5.6.99.
+ if ( defined( 'CURLOPT_SAFE_UPLOAD' ) ) {
+ $this->curlOptions[CURLOPT_SAFE_UPLOAD] = true;
+ } else if ( is_array( $postData ) ) {
+ // In PHP 5.2 and later, '@' is interpreted as a file upload if POSTFIELDS
+ // is an array, but not if it's a string. So convert $req['body'] to a string
+ // for safety.
+ $postData = wfArrayToCgi( $postData );
+ }
+ $this->curlOptions[CURLOPT_POSTFIELDS] = $postData;
+
// Suppress 'Expect: 100-continue' header, as some servers
// will reject it with a 417 and Curl won't auto retry
// with HTTP 1.0 fallback
diff --git a/includes/libs/MultiHttpClient.php b/includes/libs/MultiHttpClient.php
index c6fa914..0e9c423 100644
--- a/includes/libs/MultiHttpClient.php
+++ b/includes/libs/MultiHttpClient.php
@@ -338,6 +338,19 @@ class MultiHttpClient {
);
} elseif ( $req['method'] === 'POST' ) {
curl_setopt( $ch, CURLOPT_POST, 1 );
+ // Don't interpret POST parameters starting with '@' as file uploads, because this
+ // makes it impossible to POST plain values starting with '@' (and causes security
+ // issues potentially exposing the contents of local files).
+ // The PHP manual says this option was introduced in PHP 5.5 defaults to true in PHP 5.6,
+ // but we support lower versions, and the option doesn't exist in HHVM 5.6.99.
+ if ( defined( 'CURLOPT_SAFE_UPLOAD' ) ) {
+ curl_setopt( $ch, CURLOPT_SAFE_UPLOAD, true );
+ } else if ( is_array( $req['body'] ) ) {
+ // In PHP 5.2 and later, '@' is interpreted as a file upload if POSTFIELDS
+ // is an array, but not if it's a string. So convert $req['body'] to a string
+ // for safety.
+ $req['body'] = wfArrayToCgi( $req['body'] );
+ }
curl_setopt( $ch, CURLOPT_POSTFIELDS, $req['body'] );
} else {
if ( is_resource( $req['body'] ) || $req['body'] !== '' ) {
--
1.9.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
2986659
Default Alt Text
T118032b.patch (4 KB)

Event Timeline