232861 - bug/comment/attachment/urls get expanded in description/title of attachment link in comment (bug in quoteUrls)

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect, P2)

Description

[Daniel de Wildt]

In bug 220773 comment 15 is an attachment created with the phrase 'comment ##'
in the description. Bugzilla creates automaticly a link from this phrase. The
html code for the link is shown in the title of the <A> tag of the new created
attachment. The quotation mark in the link closes the quotation mark of the title.

Comment 1

[Daniel de Wildt]

bug 220773 comment 17 shows the link correct.

Comment 2

[Dave Miller [:justdave]]

Attachment: foo for comment 1 <script>alert("foo!")</script>

Comment 3

[Dave Miller [:justdave]]

whew.  everything gets quoted at least. :)  Was worried for a minute we had
anothe XSS bug on our hands.

Comment 4

[Dave Miller [:justdave]]

*** Bug 234259 has been marked as a duplicate of this bug. ***

Comment 5

[Bradley Baetz (:bbaetz)]

See my comments to caillon in the other bug.

Comment 6

[Dave Miller [:justdave]]

*** Bug 234160 has been marked as a duplicate of this bug. ***

Comment 7

[Dave Miller [:justdave]]

*** Bug 234419 has been marked as a duplicate of this bug. ***

Comment 8

[Gervase Markham [:gerv]]

Rewriting title to try and make it more easily findable by the people who are
filing dupes :-)

Gerv

Comment 9

[Dave Miller [:justdave]]

But "bug" isn't the only thing that get's expanded, per the rest of the comments
on this bug, so that's inaccurate.  Changing summary again.

Comment 10

[Dave Miller [:justdave]]

*** Bug 234709 has been marked as a duplicate of this bug. ***

Comment 11

[Dave Miller [:justdave]]

.

Comment 12

[Gervase Markham [:gerv]]

bbaetz said:

"I think that we need do do the @things stuff. Replacement text in those regexps
cannot ever expand to somethign that would be matched by a larger regexp.

You can't use @things itself, because then we can't put it back - you need to
use a new array/substitution bit."


What is "the @things stuff"? Is there a decent fix for this for 2.18, or does it
require a total rewrite of that munging code?

Gerv

Comment 13

[Bradley Baetz (:bbaetz)]

see @things in the relevent code. The issue is that we need to pul out and then
somehow readd the substitution back in, without doign the double substitution
which is happening now.

Comment 14

[Dave Miller [:justdave]]

*** Bug 239033 has been marked as a duplicate of this bug. ***

Comment 15

[Joel Peshkin]

Attachment: Patch - use @things to prevent double-interpretation

Comment 16

[Joel Peshkin]

Comment on attachment 147967 [details] [diff] [review]
Patch - use @things to prevent double-interpretation

The @things array is a list of numeric placeholders that look like
\0\0number\0\0

At the very end, all the instances of \0\0number\0\0 get replaced with whatever
they were placeholding for.  This is now extended to cover attachment
linkification so that bug and comment linkification (which comes later) does
not see the word bug or comment in attachment autolinks.

Comment 17

[Bradley Baetz (:bbaetz)]

See my comment which gerv quoted:

> You can't use @things itself, because then we can't put it back - you need to
> use a new array/substitution bit.

Hopwever, I can't now remember what the issue was.. I'll have to think about it

Comment 18

[Joel Peshkin]

I cannot see why the same list would be an issue.   After all, count continues
to increment, so...

bug 232861
http://bugzilla.mozilla.org
attachment 147967 [details] [diff] [review]
mailto:[e-mail]
http://localhost/bug#232861
bug 1

becomes
bug 232861
\0\00\0\0
\0\03\0\0
\0\01\0\0
\0\0\2\0\0
bug 1

and then substitues back

Comment 19

[Bradley Baetz (:bbaetz)]

Neither can I at the moment. Hmm

Comment 20

[timeless]

Comment on attachment 147967 [details] [diff] [review]
Patch - use @things to prevent double-interpretation

>+              ~($things[$count++] = GetAttachmentLink($2, $1)) &&
>+               ("\0\0" . ($count-1) . "\0\0")

I wonder if that syntax could be globally replaced w/ something like:
+	       ~($things[$count] = GetAttachmentLink($2, $1)) &&
+		("\0\0" . ($count++) . "\0\0")

but, that's food for another bug.

Comment 21

[Dave Miller [:justdave]]

looks good to me, too.

Comment 22

[Joel Peshkin]

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.266; previous revision: 1.265
done

Comment 23

[Jean Qin]

I'm using bugzilla 2.22 and having the same issue.