gunnar: server/patches/horde-webmail/1.2.0/tg t_framework_H_JS_Form_FixFormSecurityForImageUploads.diff, NONE, 1.1.2.1 series, 1.5.2.10, 1.5.2.11
cvs at kolab.org
cvs at kolab.org
Tue Nov 10 18:33:27 CET 2009
Author: gunnar
Update of /kolabrepository/server/patches/horde-webmail/1.2.0/tg
In directory doto:/tmp/cvs-serv22121/patches/horde-webmail/1.2.0/tg
Modified Files:
Tag: kolab_2_2_branch
series
Added Files:
Tag: kolab_2_2_branch
t_framework_H_JS_Form_FixFormSecurityForImageUploads.diff
Log Message:
SECURITY: Fixed image upload form issue.
--- NEW FILE: t_framework_H_JS_Form_FixFormSecurityForImageUploads.diff ---
From: Gunnar Wrobel <p at rdus.de>
Subject: [PATCH] t/framework/H/JS/Form/FixFormSecurityForImageUploads
SECURITY: Fix vulnerability in image form fields that allows overwriting
of arbitrary local files.
Signed-off-by: Gunnar Wrobel <p at rdus.de>
---
horde-webmail/lib/Horde/Form.php | 95 +++++++++++++++++------
horde-webmail/lib/Horde/UI/VarRenderer/html.php | 20 +----
2 files changed, 76 insertions(+), 39 deletions(-)
diff --git a/horde-webmail/lib/Horde/Form.php b/horde-webmail/lib/Horde/Form.php
index c8547cd..a5f13dc 100644
--- a/horde-webmail/lib/Horde/Form.php
+++ b/horde-webmail/lib/Horde/Form.php
@@ -1773,7 +1773,14 @@ class Horde_Form_Type_image extends Horde_Form_Type {
*
* @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)
{
@@ -1785,7 +1792,7 @@ class Horde_Form_Type_image extends Horde_Form_Type {
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. */
@@ -1796,25 +1803,24 @@ class Horde_Form_Type_image extends Horde_Form_Type {
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 {
@@ -1822,11 +1828,11 @@ class Horde_Form_Type_image extends Horde_Form_Type {
$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;
}
@@ -1837,11 +1843,11 @@ class Horde_Form_Type_image extends Horde_Form_Type {
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;
@@ -1896,7 +1902,7 @@ class Horde_Form_Type_image extends Horde_Form_Type {
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';
@@ -1904,19 +1910,22 @@ class Horde_Form_Type_image extends Horde_Form_Type {
/* 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. */
@@ -1927,8 +1936,8 @@ class Horde_Form_Type_image extends Horde_Form_Type {
}
/* 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);
+ move_uploaded_file($this->_img['img']['file'], $tmp_file);
+ $this->_img['img']['file'] = basename($tmp_file);
/* Store the uploaded image file data to the hidden field. */
$upload['img'] = serialize($this->_img);
@@ -1936,10 +1945,19 @@ class Horde_Form_Type_image extends Horde_Form_Type {
} 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['error'])) {
+ $this->_uploaded = PEAR::raiseError($this->_img['error']);
+ }
}
}
+ if (isset($this->_img['img'])) {
+ $_SESSION['horde_form'][$this->getRandomId()] = $this->_img['img'];
+ }
}
function getUploadedFileType($field)
@@ -1990,6 +2008,27 @@ class Horde_Form_Type_image extends Horde_Form_Type {
}
/**
+ * 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;
@@ -2011,10 +2050,18 @@ class Horde_Form_Type_image extends Horde_Form_Type {
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.
*/
diff --git a/horde-webmail/lib/Horde/UI/VarRenderer/html.php b/horde-webmail/lib/Horde/UI/VarRenderer/html.php
index 40e430f..6063864 100644
--- a/horde-webmail/lib/Horde/UI/VarRenderer/html.php
+++ b/horde-webmail/lib/Horde/UI/VarRenderer/html.php
@@ -157,10 +157,7 @@ class Horde_UI_VarRenderer_html extends Horde_UI_VarRenderer {
function _renderVarInput_image($form, &$var, &$vars)
{
$varname = htmlspecialchars($var->getVarName());
- $image = $var->getValue($vars);
-
- /* Check if existing image data is being loaded. */
- $var->type->loadImageData($image);
+ $image = $var->type->getImage($vars, $var);
Horde::addScriptFile('image.js', 'horde', true);
$graphics_dir = $GLOBALS['registry']->getImageDir('horde');
@@ -170,13 +167,11 @@ class Horde_UI_VarRenderer_html extends Horde_UI_VarRenderer {
/* Check if there is existing img information stored. */
if (isset($image['img'])) {
- /* Hidden tag to store the preview image filename. */
+ /* Hidden tag to store the preview image id. */
$html = sprintf('<input type="hidden" name="%s" id="%s" value="%s" />',
- $varname . '[img]',
- $varname . '[img]',
- @htmlspecialchars($image['img'], ENT_QUOTES, $this->_charset));
- /* Unserialize the img information to get the full array. */
- $image['img'] = @unserialize($image['img']);
+ $varname . '[hash]',
+ $varname . '[hash]',
+ $var->type->getRandomId());
}
/* Output MAX_FILE_SIZE parameter to limit large files. */
@@ -1136,11 +1131,6 @@ EOT;
/* Check if existing image data is being loaded. */
$var->type->loadImageData($image);
- if (isset($image['img'])) {
- /* Unserialize the img information to get the full array. */
- $image['img'] = @unserialize($image['img']);
- }
-
if (empty($image['img'])) {
return '';
}
--
tg: (ecde0d9..) t/framework/H/JS/Form/FixFormSecurityForImageUploads (depends on: t/kronolith/HK/GW/FixWeeklyRecurringEvents)
--
TOPGIT patch commit log
=======================
commit e6faeaf99ac92e1d3ad4ac7b7a4ad93bf99f5392
Author: Gunnar Wrobel <p at rdus.de>
Date: Tue Nov 10 18:01:37 2009 +0100
Fix patch.
commit 7bedc614e8d7a953e9fed9e1aec26b1e9b6bd3cb
Author: Gunnar Wrobel <p at rdus.de>
Date: Tue Nov 10 17:22:32 2009 +0100
SECURITY: Fix vulnerability in image form fields that allows overwriting
of arbitrary local files.
Index: series
===================================================================
RCS file: /kolabrepository/server/patches/horde-webmail/1.2.0/tg/Attic/series,v
retrieving revision 1.5.2.10
retrieving revision 1.5.2.11
diff -u -d -r1.5.2.10 -r1.5.2.11
--- series 10 Nov 2009 09:59:07 -0000 1.5.2.10
+++ series 10 Nov 2009 17:33:25 -0000 1.5.2.11
@@ -75,3 +75,4 @@
t_kronolith_HK_GW_ExportEventList.diff -p1
t_kronolith_HK_UV_dateInputFieldOrder.diff -p1
t_kronolith_HK_GW_FixWeeklyRecurringEvents.diff -p1
+t_framework_H_JS_Form_FixFormSecurityForImageUploads.diff -p1
More information about the commits
mailing list