[pkg-horde] Bug#547318: horde3: CVE-2009-3236 possibility to overwrite arbitrary files with the permissions of the webserver
Nico Golde
nion at debian.org
Fri Sep 18 15:18:14 UTC 2009
Source: horde3
Severity: grave
Tags: security patch
Hi,
the following CVE (Common Vulnerabilities & Exposures) id was
published for horde3.
CVE-2009-3236[0]:
| Within the Horde Application Framework a special kind of form element
| exists that handles image file uploads. This form element if for
| example used within the Turba address book application. These form
| elements usually move the uploaded image to some temporary file with
| a random name and verify that the file is indeed an image.
|
| Furthermore the Horde_Form_Type_image form element contains a feature
| that is meant to allow reusing the same temporary filename on reuploads.
| In order to support this the previously used temporary filename is
| stored inside hidden form fields and which is then trusted during
| upload processing.
|
| /* Get any existing values for the image upload field. */
| $upload = $vars->get($var->getVarName());
| $upload['img'] = @unserialize($upload['img']);
|
| /* Get the temp file if already one uploaded, otherwise create a
| * new temporary file. */
| if (!empty($upload['img']['file'])) {
| $tmp_file = Horde::getTempDir() . '/' . $upload['img']['file'];
| } else {
| $tmp_file = Horde::getTempFile('Horde', false);
| }
|
| /* Move the browser created temp file to the new temp file. */
| move_uploaded_file($this->_img['file'], $tmp_file);
| $this->_img['file'] = basename($tmp_file);
|
| The code snippet above demonstrates how the previously used temporary
| filename is extracted from the user supplied serialized array and then
| used as new temporary filename. It should be obvious that this allows
| writing to any writable file on the webserver. Additionally the code
| only remembers the basename() of the filename which does not contain
| the path. Therefore the later attempt to delete invalid images fails.
|
| Aside from the file overwrite problem the second problem is that the
| PHP function unserialize() is used on user supplied input which can
| be used for other things like crashing PHP through deeply nested array
| structures.
This information is from the reporter advisory, the cve id description
isn't yet online. I attached the upstream patch that was used to fix
this issue. Please test it if you use it.
If you fix the vulnerability please also make sure to include the
CVE id in your changelog entry.
For further information see:
[0] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2009-3236
http://security-tracker.debian.net/tracker/CVE-2009-3236
--
Nico Golde - http://www.ngolde.de - nion at jabber.ccc.de - GPG: 0xA0A0AAAA
For security reasons, all text in this mail is double-rot13 encrypted.
-------------- next part --------------
diff -Nurad horde-3.2.4/lib/Horde/Form.php horde-3.2.5/lib/Horde/Form.php
--- horde-3.2.4/lib/Horde/Form.php 2009-01-27 16:14:18.000000000 +0100
+++ horde-3.2.5/lib/Horde/Form.php 2009-09-14 10:10:30.000000000 +0200
@@ -1648,7 +1648,14 @@
*
* @var array
*/
- var $_img = array();
+ var $_img;
+
+ /**
+ * A random id that identifies the image information in the session data.
+ *
+ * @var string
+ */
+ var $_random;
function init($show_upload = true, $show_keeporig = false, $max_filesize = null)
{
@@ -1660,7 +1667,7 @@
function onSubmit(&$var, &$vars)
{
/* Get the upload. */
- $this->_getUpload($vars, $var);
+ $this->getImage($vars, $var);
/* If this was done through the upload button override the submitted
* value of the form. */
@@ -1671,25 +1678,24 @@
function isValid(&$var, &$vars, $value, &$message)
{
- $field = $vars->get($var->getVarName());
-
/* Get the upload. */
- $this->_getUpload($vars, $var);
+ $this->getImage($vars, $var);
+ $field = $vars->get($var->getVarName());
/* The upload generated a PEAR Error. */
if (is_a($this->_uploaded, 'PEAR_Error')) {
/* Not required and no image upload attempted. */
- if (!$var->isRequired() && empty($field['img']) &&
+ if (!$var->isRequired() && empty($field['hash']) &&
$this->_uploaded->getCode() == UPLOAD_ERR_NO_FILE) {
return true;
}
if (($this->_uploaded->getCode() == UPLOAD_ERR_NO_FILE) &&
- empty($field['img'])) {
+ empty($field['hash'])) {
/* Nothing uploaded and no older upload. */
$message = _("This field is required.");
return false;
- } elseif (!empty($field['img'])) {
+ } elseif (!empty($field['hash'])) {
/* Nothing uploaded but older upload present. */
return true;
} else {
@@ -1697,11 +1703,11 @@
$message = $this->_uploaded->getMessage();
return false;
}
- } elseif (empty($this->_img['size'])) {
+ } elseif (empty($this->_img['img']['size'])) {
$message = _("The image file size could not be determined or it was 0 bytes. The upload may have been interrupted.");
return false;
} elseif ($this->_max_filesize &&
- $this->_img['size'] > $this->_max_filesize) {
+ $this->_img['img']['size'] > $this->_max_filesize) {
$message = sprintf(_("The image file was larger than the maximum allowed size (%d bytes)."), $this->_max_filesize);
return false;
}
@@ -1712,11 +1718,11 @@
function getInfo(&$vars, &$var, &$info)
{
/* Get the upload. */
- $this->_getUpload($vars, $var);
+ $this->getImage($vars, $var);
/* Get image params stored in the hidden field. */
$value = $var->getValue($vars);
- $info = $this->_img;
+ $info = $this->_img['img'];
if (empty($info['file'])) {
unset($info['file']);
return;
@@ -1771,7 +1777,7 @@
if ($this->_uploaded === true) {
/* A file has been uploaded on this submit. Save to temp dir for
* preview work. */
- $this->_img['type'] = $this->getUploadedFileType($varname . '[new]');
+ $this->_img['img']['type'] = $this->getUploadedFileType($varname . '[new]');
/* Get the other parts of the upload. */
require_once 'Horde/Array.php';
@@ -1779,19 +1785,22 @@
/* Get the temporary file name. */
$keys_path = array_merge(array($base, 'tmp_name'), $keys);
- $this->_img['file'] = Horde_Array::getElement($_FILES, $keys_path);
+ $this->_img['img']['file'] = Horde_Array::getElement($_FILES, $keys_path);
/* Get the actual file name. */
- $keys_path= array_merge(array($base, 'name'), $keys);
- $this->_img['name'] = Horde_Array::getElement($_FILES, $keys_path);
+ $keys_path = array_merge(array($base, 'name'), $keys);
+ $this->_img['img']['name'] = Horde_Array::getElement($_FILES, $keys_path);
/* Get the file size. */
- $keys_path= array_merge(array($base, 'size'), $keys);
- $this->_img['size'] = Horde_Array::getElement($_FILES, $keys_path);
+ $keys_path = array_merge(array($base, 'size'), $keys);
+ $this->_img['img']['size'] = Horde_Array::getElement($_FILES, $keys_path);
/* Get any existing values for the image upload field. */
$upload = $vars->get($var->getVarName());
- $upload['img'] = @unserialize($upload['img']);
+ if (!empty($upload['hash'])) {
+ $upload['img'] = $_SESSION['horde_form'][$upload['hash']];
+ unset($_SESSION['horde_form'][$upload['hash']]);
+ }
/* Get the temp file if already one uploaded, otherwise create a
* new temporary file. */
@@ -1802,19 +1811,21 @@
}
/* Move the browser created temp file to the new temp file. */
- move_uploaded_file($this->_img['file'], $tmp_file);
- $this->_img['file'] = basename($tmp_file);
-
- /* Store the uploaded image file data to the hidden field. */
- $upload['img'] = serialize($this->_img);
- $vars->set($var->getVarName(), $upload);
+ move_uploaded_file($this->_img['img']['file'], $tmp_file);
+ $this->_img['img']['file'] = basename($tmp_file);
} elseif ($this->_uploaded) {
/* File has not been uploaded. */
$upload = $vars->get($var->getVarName());
- if ($this->_uploaded->getCode() == 4 && !empty($upload['img'])) {
- $this->_img = @unserialize($upload['img']);
+ if ($this->_uploaded->getCode() == 4 &&
+ !empty($upload['hash']) &&
+ isset($_SESSION['horde_form'][$upload['hash']])) {
+ $this->_img['img'] = $_SESSION['horde_form'][$upload['hash']];
+ unset($_SESSION['horde_form'][$upload['hash']]);
}
}
+ if (isset($this->_img['img'])) {
+ $_SESSION['horde_form'][$this->getRandomId()] = $this->_img['img'];
+ }
}
function getUploadedFileType($field)
@@ -1865,6 +1876,27 @@
}
/**
+ * Returns the current image information.
+ *
+ * @return array The current image hash.
+ */
+ function getImage($vars, $var)
+ {
+ $this->_getUpload($vars, $var);
+ if (!isset($this->_img)) {
+ $image = $vars->get($var->getVarName());
+ if ($image) {
+ $this->loadImageData($image);
+ if (isset($image['img'])) {
+ $this->_img = $image;
+ $_SESSION['horde_form'][$this->getRandomId()] = $this->_img['img'];
+ }
+ }
+ }
+ return $this->_img;
+ }
+
+ /**
* Loads any existing image data into the image field. Requires that the
* array $image passed to it contains the structure:
* $image['load']['file'] - the filename of the image;
@@ -1886,10 +1918,18 @@
fclose($fd);
}
- $image['img'] = serialize(array('file' => $image['load']['file']));
+ $image['img'] = array('file' => $image['load']['file']);
unset($image['load']);
}
+ function getRandomId()
+ {
+ if (!isset($this->_random)) {
+ $this->_random = uniqid(mt_rand());
+ }
+ return $this->_random;
+ }
+
/**
* Return info about field type.
*/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/pkg-horde-hackers/attachments/20090918/3f90b9da/attachment.pgp>
More information about the pkg-horde-hackers
mailing list