see attachment.

CommentFileSizeAuthor
#1 uc_payment_466438.patch1.05 KBgrendzy
uc_payment_html_watchdog.png57.96 KBgrendzy

Comments

grendzy’s picture

grendzy commented
Status: Active » Needs review
StatusFileSize
new uc_payment_466438.patch1.05 KB

Patch:

rszrama’s picture

rszrama commented

Hmm, I don't know that we should just trust whatever gets reported to that URL in case someone tries to attack the site through a payment notification or something. Perhaps use check_markup() instead? Also, feel free to fix the typo in the comment above that line. ; )

grendzy’s picture

grendzy commented

In this particular case, the actual remote input has already been checked via t() calls using the '@' (check_plain) placeholder. In practice though it might be hard to guarantee that this is applied consistently.

check_markup() is tricky, because we can't predict what the default input format will be. filter_xss() would be a good choice I think, though we'd also have to change all the <b> tags to <strong>.

rszrama’s picture

rszrama commented
Assigned: Unassigned » rszrama
Issue tags: +ubercamp sprint
Island Usurper’s picture

Island Usurper commented
Status: Needs review » Needs work

I think it's more appropriate for the Payment module to escape the messages than the individual gateways. I noticed that uc_paypal_wpp_charge() doesn't escape it's message, nor does uc_cybersource_charge(). (Sometimes. I don't know why some things are escaped and some aren't.)

To really fix this issue right, all of the payment gateway modules need to be consistent. I'd rather assume that they won't escape the message than that they will. That way being wrong isn't as big a deal.

tr’s picture

tr commented
Version: 6.x-2.x-dev » 7.x-3.x-dev
Component: Code » Payment
Assigned: rszrama » Unassigned

@rszrama: I don't think you're planning to work on this anymore?

Moving to 7.x-3.x ...

rszrama’s picture

rszrama commented

hehe Yeah, I doubt it. : P