There is a double encoding or escaping of the cc_owner field when printed on the order review page. I'm not entirely sure where the 2nd one is, but changing a check_plain() to filter_xss() in uc_credit fixes it, though it may need to be done elsewhere. If you hit the back button provided, the 'card owner' data is again double encoded when displayed in the input box. There is no need for any escaping/encoding of it at all in that case - it should just be displayed as is as it is an input field. To be honest, ubercart should be following Drupal's policy of sanitization on output, not input, so doing a check_plain on $_POST variables, etc is just wrong and leads to problems like these.

FYI, the data that triggered the double encoding was a name like "John O'Brien" - the single quote is converted to '

CommentFileSizeAuthor
double_encoding.patch849 bytesstella
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave’s picture

I'm pretty sure that all the check_plain() calls on the $_POST variables in uc_payment_method_credit() ops 'cart-process' and 'edit-process' should be removed. As explained above, there's no need to filter them here, check_plain() should be called at output time instead, and there's a very limited set of places this sensitive data should ever be displayed anyway.

longwave’s picture

Status: Needs review » Needs work

Needs work removing unnecessary check_plain()s as per #1

longwave’s picture

Status: Needs work » Fixed

Fixed in http://drupalcode.org/project/ubercart.git/commitdiff/2cdd716, also fixed some undefined index notices at the same time.

longwave’s picture

Version: 6.x-2.4 » 7.x-3.x-dev
Status: Fixed » Patch (to be ported)

This also needs looking at in 7.x, I think.

longwave’s picture

Version: 7.x-3.x-dev » 6.x-2.4
Status: Patch (to be ported) » Fixed

Actually, this seems okay in 7.x.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.