I did a quick security review of the uc_realex module.
$output .= '<h3>'. t('Reason provided: ') . $message .'</h3>';
$message is coming straight from $_POST and should be sanitized before being displayed to prevent XSS.
$timestamp = $_POST['TIMESTAMP'];
$result = $_POST['RESULT'];
$orderid = $_POST['ORDER_ID'];
$message = $_POST['MESSAGE'];
$authcode = $_POST['AUTHCODE'];
$pasref = $_POST['PASREF'];
$realexsha1 = $_POST['SHA1HASH'];
$batch = $_POST['BATCHID'];
looks suspicious. Besides $message (see above), these don't seem to be displayed anywhere else in the module but it might be a good measure to check_plain them unless you're expecting some HTML in them... just in case ubercart displays these without sanitizing them (which it should not in theory).
This is not a bug, but just a remark:
watchdog('uc_realex', t('Receiving new order notification for order !order_id.', array('!order_id' => check_plain($_POST['ORDER_ID']))));
the @ placeholder takes care of the check_plain, I would rewrite this as
watchdog('uc_realex', t('Receiving new order notification for order @order_id.', array('@order_id' => $_POST['ORDER_ID'])));
(there are more than one occurence in uc_realex.module)
See http://drupal.org/writing-secure-code for more tips.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | 667464.patch | 1.99 KB | stella |
Comments
Comment #1
stella commentedComment #2
alanburke commentedFixed in HEAD.
Thanks guys!