474394 - Encode more characters in url_encode()

Status: RESOLVED FIXED

(Webtools Graveyard :: Tinderbox, defect)

Description

[Reed Loden [:reed]]

Attachment: patch - v1

In tbglobals.pl's url_encode(), we should encode more characters to help prevent XSS. Bonsai might need this same patch.

Comment 2

[Reed Loden [:reed]]

Comment on attachment 357769 [details] [diff] [review]
patch - v1

cls, I'll take your review, too... didn't know you were around. :)

Comment 3

[timeless]

wouldn't it make more sense to just use a simple regexp pattern?

Comment 4

[Reed Loden [:reed]]

(In reply to comment #3)
> wouldn't it make more sense to just use a simple regexp pattern?

What do you mean?

Comment 5

[cls]

I'm sure that he means that It Would Be Nice(tm) if we could use a regexp or pattern like the one being used for url_decode() instead of adding a lot of individual exceptions.  I looked for one but couldn't find it.

http://mxr.mozilla.org/mozilla/source/webtools/tinderbox/tbglobals.pl#390

Comment 6

[Reed Loden [:reed]]

RFC 3986 gives a specific list of what should and shouldn't be encoded, so we can't just use a regex to encode everything, just like url_decode() is able just to decode anything encoded. This has to be an actual list of characters to be encoded, sadly.

Comment 7

[Reed Loden [:reed]]

Thanks for the review.

Checking in tbglobals.pl;
/cvsroot/mozilla/webtools/tinderbox/tbglobals.pl,v  <--  tbglobals.pl
new revision: 1.70; previous revision: 1.69
done

Comment 8

[cls]

Turns out that bonsai had the regexp I was looking for.

http://mxr.mozilla.org/mozilla/source/webtools/bonsai/CGI.pl#42

Comment 9

[cls]

patch v1 checked into tbox1_20080527_cls_branch