Skip to content

Commit 288f81c

Browse files
committed
Cross-Site Request Forgery (CSRF) --------------------------------- Abdullah Hussam Gazi discovered that the CSRF protection mechanism introduced a few years ago to secure the forms generated with the HTML_Quickform library (most fo the forms in Revive Adserver's admin UI) could be easily bypassed by sending an empty token along with the POST data. The range of malicious actions include, but is not limited to, modifying entities like banners and zones and altering preferences and settings. CWE: CWE-352 CVSSv2: 5.1 (AV:N/AC:H/Au:N/C:P/I:P/A:P)
1 parent 12cefa6 commit 288f81c

File tree

3 files changed

+14
-6
lines changed

3 files changed

+14
-6
lines changed

lib/OA/Admin/UI/component/Form.php

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,11 @@ function __construct($formName='', $method='POST', $action='', $target='', $attr
125125
//trim spaces from all data sent by the user
126126
$this->applyFilter('__ALL__', 'trim');
127127

128-
$this->addElement('hidden', 'token', phpAds_SessionGetToken());
129-
$this->addRule('token', 'Invalid request token', 'callback', 'phpAds_SessionValidateToken');
128+
if (!defined('phpAds_installing')) {
129+
$this->addElement('hidden', 'token', phpAds_SessionGetToken());
130+
$this->addRule('token', 'Missing request token', 'required');
131+
$this->addRule('token', 'Invalid request token', 'callback', 'phpAds_SessionValidateToken');
132+
}
130133
}
131134

132135
function validate()
@@ -136,7 +139,7 @@ function validate()
136139
if (!$ret) {
137140
// The form returned an error. We need to generate a new CSRF token, in any.
138141
$token = $this->getElement('token');
139-
if (!empty($token)) {
142+
if (!empty($token) && !PEAR::isError($token)) {
140143
$token->setValue(phpAds_SessionGetToken());
141144
}
142145
}

lib/pear/HTML/QuickForm/Renderer/Array.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,13 +225,16 @@ function renderElement(&$element, $required, $error)
225225
} // end func renderElement
226226

227227

228-
function renderHidden(&$element)
228+
function renderHidden(&$element, $required, $error)
229229
{
230230
if ($this->_collectHidden) {
231231
$this->_ary['hidden'] .= $element->toHtml() . "\n";
232232
} else {
233233
$this->renderElement($element, false, null);
234234
}
235+
if (!empty($error)) {
236+
$this->_ary['errors'][$elAry['name']] = $error;
237+
}
235238
} // end func renderHidden
236239

237240

lib/pear/HTML/QuickForm/hidden.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,14 @@ function freeze()
7979
* Accepts a renderer
8080
*
8181
* @param HTML_QuickForm_Renderer renderer object
82+
* @param bool Whether an element is required
83+
* @param string An error message associated with an element
8284
* @access public
8385
* @return void
8486
*/
85-
function accept(&$renderer)
87+
function accept(&$renderer, $required=false, $error=null)
8688
{
87-
$renderer->renderHidden($this);
89+
$renderer->renderHidden($this, $required, $error);
8890
} // end func accept
8991

9092
// }}}

0 commit comments

Comments
 (0)