Closed Bug 206037 Opened 21 years ago Closed 18 years ago

[SECURITY] Fix escaping/quoting in editXXX.cgi scripts

Categories

(Bugzilla :: Administration, task)

2.10
task
Not set
minor

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: gerv, Assigned: LpSolit)

References

Details

(Whiteboard: ready for 2.18.6] [ready for 2.20.3][ready for 2.22.1][ready for 2.23.3])

Attachments

(4 files, 25 obsolete files)

43.73 KB, patch
justdave
: review+
Details | Diff | Splinter Review
41.29 KB, patch
justdave
: review+
Details | Diff | Splinter Review
42.34 KB, patch
justdave
: review+
Details | Diff | Splinter Review
53.38 KB, patch
justdave
: review+
Details | Diff | Splinter Review
If you add a component, the name isn't escaped properly, or is over-escaped, or
something.

[This bug report is a transcription of a scrap of paper. I assume that if
someone tests Add Component a little bit (with & and & signs etc.) they'll
figure out what the problem was that I saw.]

Gerv
It is true that very few (if any) of the components and products are
html_quote()-ed in both editproducts.cgi and editcomponents.cgi.
Obviously, the proper templatisation of the admin pages is far preferable, but
would people be interested in a patch to the current
edit{products,components}.cgi pages to add html_quote() calls where necessary?
yes.  At least for 2.16.4.  We ought to make them run in taint mode while we're
at it.  That'll make things easier for the win32 folks.

Zippy has all the edit*.cgi files already templatized, but we'll have do our own
templates, theirs depend on too much javascript foo, not to mention they've been
hacked to pieces to handle their project setup.  So I don't know how quickly it
can be done.  We might as well do the html_quote stuff for 2.17.5 too, as a
stop-gap measure.

Several of us have known about the XSS problems in edit*.cgi for a while now,
and we've kind of been shoving it aside because it requires admin privs to
exploit it, and we figure the admins should know better.  In truth, Bugzilla is
starting to be used more and more in situations where the admin winds up being
someone who's not exactly tech savvy, and they can accidently shoot themselves
with it, so we might as well get it over with and fix it :)
Group: webtools-security
Whiteboard: [wanted for 2.16.4] [wanted for 2.17.5]
I'll attach a patch sometime this week.
Assignee: justdave → bugzilla
Status: NEW → ASSIGNED
Summary: Bad escaping in Add Component → Fix escaping/quoting in editXXX.cgi scripts
Running edit*.cgi in taint mode is bug#141006.
Version 1, adding quoting to the editXXX.cgi scripts. Currently, only the
keyword description is not quoted, as that was explicit in keywords.html.tmpl.
Attachment #129127 - Flags: review?
Target Milestone: --- → Bugzilla 2.16
Whiteboard: [wanted for 2.16.4] [wanted for 2.17.5] → [wanted for 2.16.5] [wanted for 2.17.6]
Comment on attachment 129127 [details] [diff] [review]
proper quoting for admin cgis,version 1

bitrotted.

This applies to the tip with only 14 conflicts.  That's actually not bad given
there's close to 150 hunks in this patch.

It's obviously trunk from 6 months ago and not 2.16 though.  It looks like we
might actually be making progress on the templatization of the admin stuff on
the trunk shortly, but something along the lines of this patch is needed for
2.16.
Attachment #129127 - Attachment is obsolete: true
Attachment #129127 - Flags: review?
Are we really going to attempt all this on 2.16? Is it not better to put the
effort into making win32 work for 2.18?

Gerv
Whiteboard: [wanted for 2.16.5] [wanted for 2.17.6] → [wanted for 2.16.5] [wanted for 2.17.7]
This doesn't look like it'll happen in the next 4 days :(
Whiteboard: [wanted for 2.16.5] [wanted for 2.17.7] → [wanted for 2.16.6] [wanted for 2.17.8]
Depends on: 235265
Whiteboard: [wanted for 2.16.6] [wanted for 2.17.8] → [wanted for 2.16.6] [wanted for 2.18rc1]
OK, we need this for 2.18 whether we have admin templatization done or not.  I'm
tired of sitting on this, and I want 2.18 to be clean :)
Flags: blocking2.18+
Flags: blocking2.16.6+
*** Bug 236142 has been marked as a duplicate of this bug. ***
OK, the following edit*.cgi files also have references to %FORM that we're
trying to eliminate as part of bug 225818:

editcomponents.cgi
editflagtypes.cgi
editgroups.cgi
editmilestones.cgi
editproducts.cgi
editusers.cgi
editversions.cgi

it's going to be really hard to fix the %FORM stuff without fixing this with it.
 Should we go ahead and file bugs for the %FORM stuff for the above files, and
kill off the XSS stuff on the trunk via those, and just do the 2.16 branch on
this bug?  Remember that on the trunk, people are expected to pick up new
features in order to get security patches, so we don't necessarily have to have
a single coherant patch for the trunk...

thoughts anyone?
Depends on: 236560
Flags: blocking2.18-
Flags: blocking2.18+
Flags: blocking2.16.7+
Flags: blocking2.16.6-
Flags: blocking2.16.6+
Whiteboard: [wanted for 2.16.6] [wanted for 2.18rc1] → [wanted for 2.16.7] [wanted for 2.18.1] [wanted for 2.19.1]
A question which arose during the editcomponents templatisation:

Should *absolutely* everything be filtered out-of-the-box?

Currently, b.m.o uses html in the descriptions of products/components/keywords
-- should these description fields be html-filtered in the Bugzilla codebase, or
is it a Bugzilla feature that the description fields for products, components,
and keywords may contain html?

I would vote that the default should be to filter everything, and a site can
customise away the filtering if they want html in these descriptions - comments?
> I would vote that the default should be to filter everything, and a site can
> customise away the filtering if they want html in these descriptions - comments?

This has backwards-compatibility issues. Perhaps we should note in the UI which
fields allow HTML and which do not, but I think it's perfectly reasonable to
allow admins to put HTML in some fields. After all, if an admin can edit the
templates, you have to trust them :-)

Gerv
Whiteboard: [wanted for 2.16.7] [wanted for 2.18.1] [wanted for 2.19.1] → [wanted for 2.16.8] [wanted for 2.18.1] [wanted for 2.20]
Flags: blocking2.16.7+ → blocking2.16.7-
I have a question about the profiles disabledtext field. This is currently
displayed without filtering when a disabled user tries to log in. Should html be
allowed in this field, or should it be filtered?
HTML should be allowed. After all, it's admin-controlled only, and they may want
to put in <a href="mailto:">s or a big red "Stuff Off".

Gerv
OK, this *might* actually be fixed on the trunk now, as a side effect of the
admin CGIs getting templatized...
Whiteboard: [wanted for 2.16.8] [wanted for 2.18.1] [wanted for 2.20] → [wanted for 2.16.10] [wanted for 2.18.2] [wanted for 2.20]
(In reply to comment #17)
> OK, this *might* actually be fixed on the trunk now, as a side effect of the
> admin CGIs getting templatized...

From what I can see, this is *not* fixed. At least not in editproducts.cgi.
I think this bug is now fixed on 2.21.1. editproducts.cgi has been templatized
and (do)editparams.cgi seem safe at first glance.
doeditparams.cgi no longer exists in 2.21.1+ and editparams.cgi has been
templatized.
Whiteboard: [wanted for 2.16.10] [wanted for 2.18.2] [wanted for 2.20] → [wanted for 2.16.11] [wanted for 2.18.5] [wanted for 2.20.1][doesn't affect 2.22]
*** Bug 319085 has been marked as a duplicate of this bug. ***
I would like to make things clear: as long as we allow at least *one* field to be unfiltered (which is the case of the description fields), we are vulnerable to XSS. html_quote()-ing everything else in 2.16, 2.18 and 2.20 except the descriptions would not completely fix the problem. And having a whitelist of allowed tags in unfiltered fields is not an easy task, per zack's comment: bug 319091 comment 6.

Look at the obsolete patch attached here. It's a 75 Kb patch, for one version of Bugzilla only! Does it really worth spending hours (days) to have a non perfect solution?

One solution could be to extend bug 281181 to all actions in edit*.cgi pages, not only on deletion. In other words:

- accept action="new" only if action="add" has been called previously
- accept action"update" only if action="edit" has been called previously
- accept action="delete" only of action="del" has been called previously

All this by using tokens.

This way, even if you click on some untrusted URL which redirects you to one of these actions, you will be stopped before anything bad is either displayed (XSS) or inserted into the DB.

Let's discuss this now, take a decision, and fix it for good!

LpSolit

PS: please let discuss this problem here. Not everybody is in the security@ mailing-list, but I still want other reviewers in the discussion. Or open a discussion in reviewers@.
In comment#13, I voted for filtering _EVERYTHING_ out of the box and allowing admins to change things locally if they want to...

Extra token checks in the admins is good as well.
Actually, the token approach has some other advantages.  It puts an infrastructure in place that enables us to prevent accidental submission of duplicate bugs.
I may be being dumb, but I can't see the security issue here. LpSolit: can you give us a sample attack scenario which doesn't start with untrusted people having admin rights?

Gerv
(In reply to comment #25)
> I may be being dumb, but I can't see the security issue here. LpSolit: can you
> give us a sample attack scenario which doesn't start with untrusted people
> having admin rights?

Precisely, I cannot! Thanks gerv! I will quote what I said in bug 319085 comment 1, which I marked as a dupe of this bug:

"The only field which is not filtered is the product description, and it's so by design. If an admin is stupid enough to inject malicious JS code here, then this Bugzilla installation doesn't worth to be considered."


That's the whole discussion: either we trust admins and stop complaining about XSS vulnerabilities in edit*.cgi pages, or we don't trust them and we have to fix the problem about unfiltered fields as described in comment 22.

My comment 22 was a reaction to the discussion in security@ about the fact that we didn't fix this bug earlier. "You estimate that there are XSS holes? ok, so let's see what we have to fix" was the summary of my previous comment.

What I want from you (all of you, CC'ed guys) is a clear decision about what to do here. Allowing *one* field unfiltered would still allow a "stupid" admin to inject malicious JS code. So we come back to the beginning of this comment: do we trust the admins or not?


gerv, I hope this answers your question.
Right, I see. I vote for trusting the admins, clearly.

However, that doesn't necessarily mean that we should leave bugs like this entirely unfixed, because they may prevent e.g. admins using & or < characters in their product names, which they might like to do.

Gerv
Yes, I think that as long as this is fixed on the tip, it's fine. If somebody can get admin access when they shouldn't be able to, that's a security bug. I mean, heck, what are we going to do next, say "Admins can delete users," and file that as a security bug? Is "root users on Linux can do 'rm -rf /'" a security bug? If you're an administrator, you can do anything. If you have DB write access, you can do anything.

The token thing is actually another bug -- bug 26257.
The token thing in bug 26257 is the main reason this is actually an issue.  Yes, we should be able to trust the admins.  No, we can't trust the people who might trick an admin into viewing a page with a hidden IFRAME on it that links to one fo the admin UIs with a URL that does something stupid, and the admin would never be the wiser because it happened in an IFRAME.  Between introducing the tokens and filtering the HTML tags that we allow in the descriptions, I think the filtering is probably going to be far less risky to backport to the older branches we're still supporting.  Many of the issues listed here are also blatant XSS holes in places we didn't intend to have HTML in the older branches (but got fixed on the tip as part of templatizing).
(In reply to comment #29)
> the tokens and filtering the HTML tags that we allow in the descriptions, I
> think the filtering is probably going to be far less risky to backport to the
> older branches we're still supporting.

So let's enumerate these HTML tags we will allow and those we will forbid:

allow: <b>, <em>, <i>, <u>, <p>, <br>, <span>, <div>, <ul>, <li>, <ol>, <dt>, <dd>, <dl> + what else? <a> to let us point to a malicious page ;)


forbid: <script>, <form>, onload, onsubmit, onfoo, onbar, ... + ???

of course, we should take care of tags of the form <p class="" foo="bar">


I suggest a new routine Util::html_light => [% description FILTER html_light %]


GavinS, is that something you want to do? If yes, please only fix missing html_quote() and implement this routine. Don't fix nits not directly related to this bug (to avoid another 75Kb patch).
Fixing this requires taint mode be enabled; its the only way to make sure that we have everything.

And since taint is runtime, and these pages aren't used that often, the first release will break something. (What it breaks was probably a security issue, though)
Hello, I'm maintaining Bugzilla Debian packages and am interested in having a valid patch for closing this issue on our stable package, which is version 2.16.7.

What does remain to be done for this bug to be closed for the 2.16 branch? 

Thanks.
(In reply to comment #32)
> What does remain to be done for this bug to be closed for the 2.16 branch? 

Adding url_quote() and html_quote() appropriately to all the admin pages. Attachment 129127 [details] [diff] is the sort of thing (but that patch does have a few changes which are not strictly for this bug). I don't remember which version that was against.
The support for the 2.16 branch is over. Also removing old flags.
Flags: blocking2.18-
Flags: blocking2.16.7-
Flags: blocking2.16.6-
Whiteboard: [wanted for 2.16.11] [wanted for 2.18.5] [wanted for 2.20.1][doesn't affect 2.22] → [wanted for 2.18.6] [wanted for 2.20.3][doesn't affect 2.22]
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
QA Contact: mattyt-bugzilla → default-qa
Attached patch patch for tip, v1 (obsolete) — Splinter Review
Implement my suggestion from comment 30. The list of HTML elements considered as safe is:

my @safe = qw(b em i u p br);

Attributes are not accepted, as well as URLs (because <a> requires at least one attribute). This is pretty restrictive, but this allows you to do some basic formatting which I consider to be enough for component/product/classification descriptions.

This patch is for 2.23.2+.
Assignee: bugzilla → LpSolit
Attachment #229914 - Flags: review?(justdave)
Attached patch patch for tip, v1.1 (obsolete) — Splinter Review
Two changes:
- Added <strong> to the whitelist;
- Filter product and component descriptions in global/choose-product.html.tmpl and reports/components.html.tmpl.

There are still two unsafe places where FILTER none is used:
1. The text for disabled user accounts;
2. The text for the new Param('announcehtml') in global/header.html.tmpl.

If it is possible to use IFRAMEs for product and component descriptions, I don't see what would prevent an attacker from using the same trick for these two fields.
Attachment #229914 - Attachment is obsolete: true
Attachment #229949 - Flags: review?(justdave)
Attachment #229914 - Flags: review?(justdave)
Hum, I was looking at filterexceptions.pl and thought: "huh?! we have tons of unfiltered fields here which are strings entered by the user" (in opposition to integers only or hardcoded strings).

In other words: the fields I fixed in my patch are only the top of the iceberg compared to all fields which are "silently" unfiltered. Why? Because keyword and group descriptions, among many others, are unfiltered too.

Now that all pages have been templatized, I would tend to mark this bug as FIXED, retarget it to 2.22, remove the security flag, and live with it. Per my comment 36, as long as we allow at least one field to be completely unfiltered, an attacker could try to inject code there.
(In reply to comment #36)
> There are still two unsafe places where FILTER none is used:
> 1. The text for disabled user accounts;

Note that this point has been filed separately as bug 319091 which has been marked as WONTFIX. So this bug no longer makes sense, IMO, as there is at least one door open.
(In reply to comment #37)
> an attacker could try to inject code there.

An admin attacker?? Preventing any attacker who isn't an admin is already covered in bug 26257.

Any pre-template Bugzilla may also have more holes of which we are unaware, which is what this bug was originally about.

However, continuing to allow this hole is basically like giving anybody with editcomponents or editkeywords the "editusers" privilege, because we basically allow them to steal users' logincookies if they so desire (via a <script>).
bug 26257 is unrelated. It's about process_bug.cgi. And no, I'm not talking about an admin attacker, see comment 29.
(In reply to comment #40)
> bug 26257 is unrelated. It's about process_bug.cgi. And no, I'm not talking
> about an admin attacker, see comment 29.

  I did see comment 29. That's exactly what I was talking about. We can fix that problem with bug 26257, as justdave mentions in that comment.
Attached patch patch for tip, v2 (obsolete) — Splinter Review
Per private discussion on IRC with mkanat and justdave, we came to the conclusion that everything which had to be filtered must be filtered, meaning removing some stuff from filterexceptions.pl. Also, to avoid attacks against fields such the disabled text, bug 281181 must also be fixed to get a complete solution.
Attachment #229949 - Attachment is obsolete: true
Attachment #230034 - Flags: review?(justdave)
Attachment #229949 - Flags: review?(justdave)
Attachment #230034 - Attachment description: patch, v2 → patch for tip, v2
Attachment #229949 - Attachment description: patch, v1.1 → patch for tip, v1.1
Let's first see if this patch is good before wasting time backporting it.
Whiteboard: [wanted for 2.18.6] [wanted for 2.20.3][doesn't affect 2.22] → [wanted for 2.18.6] [wanted for 2.20.3][wanted for 2.22.1] patch for tip awaiting review
What I hate with security bugs is that as you have to wait the next release before committing patches, this gives you a plently of time to see them bitrotten.
Attachment #230034 - Attachment is obsolete: true
Attachment #230568 - Flags: review?(justdave)
Attachment #230034 - Flags: review?(justdave)
Attached patch patch for 2.22, v1 (obsolete) — Splinter Review
Attachment #231163 - Flags: review?(justdave)
Attached patch patch for tip, v2.1 (obsolete) — Splinter Review
Fix admin/groups/edit.html.tmpl (I found it while backporting it on 2.22). My patch for 2.22 already contains this fix.
Attachment #230568 - Attachment is obsolete: true
Attachment #231165 - Flags: review?(justdave)
Attachment #230568 - Flags: review?(justdave)
Attached patch patch for 2.20, v1 (obsolete) — Splinter Review
Attachment #231227 - Flags: review?(justdave)
Attached patch patch for tip, v2.2 (obsolete) — Splinter Review
I forgot to remove a "allow_html_content => 1" from admin/users/list.html.tmpl which is now useless. The patch for 2.20 already contains this fix.
Attachment #231165 - Attachment is obsolete: true
Attachment #231228 - Flags: review?(justdave)
Attachment #231165 - Flags: review?(justdave)
Attached patch patch for 2.22, v1.1 (obsolete) — Splinter Review
Same fix as above about admin/users/list.html.tmpl.
Attachment #231163 - Attachment is obsolete: true
Attachment #231229 - Flags: review?(justdave)
Attachment #231163 - Flags: review?(justdave)
Attached patch patch for 2.18, v1 (obsolete) — Splinter Review
This one was really painful to write and to test. Note that for these fixes to be really complete, we should also fix bug 330555.
Attachment #231266 - Flags: review?(justdave)
Attached patch patch for 2.20, v1.1 (obsolete) — Splinter Review
Forgot to escape milestones and versions when listing all products. The patch for 2.18 already contains this fix. 2.22 and tip are not affected. So this should be the last patch. :)
Attachment #231227 - Attachment is obsolete: true
Attachment #231268 - Flags: review?(justdave)
Attachment #231227 - Flags: review?(justdave)
Blocks: 346524
Blocks: 346525
No longer blocks: 346524
Flags: blocking2.22.1?
Flags: blocking2.20.3?
Flags: blocking2.18.6?
Flags: blocking2.22.1?
Flags: blocking2.22.1+
Flags: blocking2.20.3?
Flags: blocking2.20.3+
Flags: blocking2.18.6?
Flags: blocking2.18.6+
Attached patch patch for 2.18, v1.1 (obsolete) — Splinter Review
I found an unfiltered field in editgroups.cgi while doing some additional tests.
Attachment #231266 - Attachment is obsolete: true
Attachment #231347 - Flags: review?(justdave)
Attachment #231266 - Flags: review?(justdave)
Blocks: 323521
Whiteboard: [wanted for 2.18.6] [wanted for 2.20.3][wanted for 2.22.1] patch for tip awaiting review → [wanted for 2.18.6] [wanted for 2.20.3][wanted for 2.22.1] patches awaiting review from justdave
Comment on attachment 231228 [details] [diff] [review]
patch for tip, v2.2

>Index: Bugzilla/Util.pm
>+sub html_light_quote {

>+    # List of the HTML elements considered as safe.
>+    # Note that none of them can have attributes, else
>+    # they would be escaped. <a> is not considered as safe.

<a> needs to be made safe or you'll break b.m.o (and other installations as well I'd imagine).  Most of the product descriptions have links to the product's home page in them.  I would suggest that we allow <a>, but only allow title and href attributes and restrict what can be linked to (well-formed URLs on common protocols only)

We don't have any on b.m.o, but I could easily imaging people using <abbr> tags in them as well.  That should be fairly safe, but we'd need to allow a title attribute on that one (I think that's the right one).

Attribute filtering is going to be a pain in the butt. :(  (And I'm sure you were hoping we could get away without it)


>Index: template/en/default/admin/products/edit-common.html.tmpl
>   <td><textarea rows="4" cols="64" wrap="virtual" name="description">
>-        [% product.description FILTER none %]</textarea>
>+        [% product.description FILTER html_light %]</textarea>

This should probably be FILTER html rather than either FILTER none or FILTER html_light.  This is the content of a textarea, so we need to neither parse the data nor corrupt it.


Other than those two things, nothing else is jumping out at me, and I'm not finding any problems anywhere.  If it weren't for the attribute thing, this would be easy to fix up. :(
Attachment #231228 - Flags: review?(justdave) → review-
(In reply to comment #53)
> I would suggest that we allow <a>, but only 
> allow title and href attributes and restrict what can be linked to
> (well-formed URLs on common protocols only)

The same list of URLs we accept for hyperlinking in comments (quoteUrls) would probably work.
In order to allow attributes, I could write:

    $text =~ s#<(($safe)(?:\s+[^<>\\]+)?)>#$chr$1$chr#go;

but it lets <a href="javascript:alert('foo')">bar</a> going through. Is that a problem?
For the record, I think we're getting into HTML cleaning, and that's a very dangerous area. Of course, I suppose it's at least more secure than not filtering them at all.
(In reply to comment #55)
> In order to allow attributes, I could write:
> 
>     $text =~ s#<(($safe)(?:\s+[^<>\\]+)?)>#$chr$1$chr#go;

It's too easy to abuse this regexp, e.g.:

<b attr=">" <!-- Hidden comment //--> >

I'm not sure this is valid HTML (besides the fact that attr is not a valid attribute of <b>), but at least I could easily break the HTML page this way. I entirely agree with Max on this, it's a dangerous area. The good news is that all unrecognized tags will be escaped, and thanks to bug 281181, it should not be possible to inject such code without the admin consent.

One alternative could be to put this regexp in a comment (i.e. rejecting all attributes, and so rejecting <a>), and write in the documentation that admins can uncomment this line if they want to allow attributes, but that this regexp could easily be abused.
Please, please don't try and strip or sanitise HTML using regexps. The authors of the Perl FAQ even say it's a bad plan:
http://perldoc.perl.org/perlfaq9.html#How-do-I-remove-HTML-from-a-string%3f

They do provide a regexp if you are set on using one, but given the non-performance-critical nature of this code, can't we use HTML::Scrubber or HTML::Sanitiser?

Gerv

(In reply to comment #58)
> can't we use HTML::Scrubber or HTML::Sanitiser?

Yes, I was reading the doc for HTML::Sanitizer and it looks very promising and easy to use. The problem is, what do we do for systems which do not have this package installed?
> Yes, I was reading the doc for HTML::Sanitizer and it looks very promising and
> easy to use. The problem is, what do we do for systems which do not have this
> package installed?

The same thing we always do - add it to the list of required packages, and to Bundle::Bugzilla :-)

Gerv
(In reply to comment #60)
> The same thing we always do - add it to the list of required packages, and to
> Bundle::Bugzilla :-)

justdave just suggested me on IRC to make this package optional and forbid attributes unless they have this package installed. So admins complaining that their links are escaped would have to install this package.
That sounds a bit more complicated. And who says attributes are the only problem - have you tested your regexp against the hard examples given in the FAQ question?

What's wrong with just requiring this package? Does it have nasty dependencies? Or native code? Or is it as simple as "Install HTML::Sanitizer"?

Gerv
(In reply to comment #62)
> That sounds a bit more complicated. And who says attributes are the only
> problem - have you tested your regexp against the hard examples given in the
> FAQ question?

No need. My example in comment 57 already broke my page. :(



> What's wrong with just requiring this package? Does it have nasty dependencies?
> Or native code? Or is it as simple as "Install HTML::Sanitizer"?

HTML::Sanitizer requires HTML::TreeBuilder, HTML::Scrubber requires HTML::Parser. So I'm now reading the doc for HTML::Scrubber as I guess HTML::Parser is more popular.

I checked on Mandriva 2006.0, and none of these HTML::Sanitizer/Scrubber packages exist in the distro. Maybe this would be a problem for admins which have to use packages from their distro only (due to some internal policy).

I don't think it's that difficult. I could do an eval {require HTML::Scrubber;}, and if it fails, then I forbid attributes using my previous patch, else pass the text to the sanitizer.
Are you sure your previous patch forbids attributes in a watertight way? And are you sure that forbidding attributes makes things completely safe? For example:

<script>
  // Do something nasty
</script>

has no attributes.

Gerv
(In reply to comment #64)
> Are you sure your previous patch forbids attributes in a watertight way?

Yes, because I explicitly enumerate allowed tags:

/<(p|br|strong|em|...)>/



> <script>
>   // Do something nasty
> </script>
> 
> has no attributes.

But <script> doesn't belong to my list of safe tags enumerated above.
title="..."
<blockquote>
<address>
<strong>
<strike>
<cite>
<ins>
<del>
<q>
i think this is a complete list.

<a><abbr><acronym><address><area><b><bdo><big><blockquote><br><caption><center><cite><code><col><colgroup><dd><del><dir><div><dfn><dl><dt><em><font><h1><h2><h3><h4><h5><h6><hr><i><img><ins><kbd><label><li><map><ol><p><pre><q><s><samp><small><span><strike><strong><sub><sup><table><tbody><td><tfoot><th><thead><tr><tt><u><ul><var>

intentionally removed are script, frames, forms, style/link/things outside body, object/applet/embed

of them, <s> was among the ones i did want to include
Attached patch patch for tip, v3 (obsolete) — Splinter Review
OK, I now use HTML::Scrubber if this package is installed. This allows us to be less restrictive and accept tags such as <a href="..." title="...">foo</a> and <span class="">bar</span>. If HTML::Scrubber is not installed, we fallback on my method, i.e. not allowing attributes, which is not that bad in most cases.
Attachment #231228 - Attachment is obsolete: true
Attachment #237791 - Flags: review?(justdave)
Attached patch patch for tip, v3.1 (obsolete) — Splinter Review
Let's avoid any attempt to abuse <q> and <blockquote>.
Attachment #237791 - Attachment is obsolete: true
Attachment #237794 - Flags: review?(justdave)
Attachment #237791 - Flags: review?(justdave)
My apache error log is full of this message:

Parsing of undecoded UTF-8 will give garbage when decoding entities at /usr/lib/perl5/site_perl/5.8.7/HTML/Scrubber.pm line 322.

The 'utf8' parameter is turned on in my bz installation, but I also have data which is encoded in ISO-8859-1 (because some data come from 2.18).

Gerv, any clue how to fix this problem?
Attached patch patch for tip, v3.2 (obsolete) — Splinter Review
Possible fix to suppress the warning in the web server error log. I have absolutely *no idea* if this hack is correct. It seems to work fine with the 'utf8' param turned on as well as turned off.
Attachment #237862 - Flags: review?(justdave)
FYI, we have to write:
if (ref($scrubber->{_p}) eq 'HTML::Parser')
instead of:
if (exists $scrubber->{_p} && $scrubber->{_p}->can('utf8_mode'))
else it fails if $scrubber->{_p} is not an object.
Attached patch patch for 2.18, v2 (obsolete) — Splinter Review
HTML::Parser::utf8_mode() only works with Perl 5.8 and higher. It seems that Perl 5.8 or higher use UTF-8 internally and so we have to pass this information to HTML::Parser. Perl 5.6.x is not affected.
Attachment #231347 - Attachment is obsolete: true
Attachment #238049 - Flags: review?(justdave)
Attachment #231347 - Flags: review?(justdave)
Attached patch patch for tip, v4 (obsolete) — Splinter Review
Attachment #237794 - Attachment is obsolete: true
Attachment #237862 - Attachment is obsolete: true
Attachment #238054 - Flags: review?(justdave)
Attachment #237794 - Flags: review?(justdave)
Attachment #237862 - Flags: review?(justdave)
Comment on attachment 238054 [details] [diff] [review]
patch for tip, v4

>+    # Is HTML::Scrubber installed?
>+    eval { require HTML::Scrubber; };
>+
>+    if ($@) { # Package not installed.

Not sure if it's worth making it optional on the trunk.  Definitely on the backports, but on the trunk I think we can just require it and be done with it.

>+        my @rules = (a => {
>+                           href  => qr{^(?!(?:java)?script:)}i,

The one thing that worries me about this is that there's other URL schemes other than javascript: that can be dangerous (such as data:), and there may be new schemes in the future which are.  I would much rather give a list of schemes that we'll allow rather than a list of the ones we won't.  It's more future-proof that way, and it's what we already do in quoteUrls (which we should be sharing the legal scheme list with - which I thought I suggested already on IRC).  It also provides an obvious place in the code for people to add a scheme if someone has something they want to use on their site that it's not allowing.
Attachment #238054 - Flags: review?(justdave) → review-
Comment on attachment 238049 [details] [diff] [review]
patch for 2.18, v2

ok, this is only r- because of the same reason given for the tip one -- we need a uri scheme whitelist rather than a blacklist on the URI attribute scrubbers.  This patch is r+ with that fixed however (but because of this being a security bug, we still need the final patch on the bug before it gets committed).
Attachment #238049 - Flags: review?(justdave) → review-
Comment on attachment 231229 [details] [diff] [review]
patch for 2.22, v1.1

This is r+ with the exception of the Scrubber stuff that hasn't been backported yet.  Just getting it here on record so you know I've looked at it and to get it out of my review queue. :)
Attachment #231229 - Flags: review?(justdave) → review-
Comment on attachment 231268 [details] [diff] [review]
patch for 2.20, v1.1

This is r+ with the exception of the Scrubber stuff that hasn't been backported yet.  Same as the 2.22 one. :)
Attachment #231268 - Flags: review?(justdave) → review-
Attached patch patch for tip, v5 (obsolete) — Splinter Review
HTML::Scrubber is still optional as it's not available in all distro yet (Mandriva only has it since Mandriva 2007 which should be released this week).

I now use a whitelist instead of a blacklist.
Attachment #238054 - Attachment is obsolete: true
Attachment #239137 - Flags: review?(justdave)
Attached patch patch for 2.22, v2 (obsolete) — Splinter Review
Use HTML::Scrubber + whitelist
Attachment #231229 - Attachment is obsolete: true
Attachment #239144 - Flags: review?(justdave)
Attached patch patch for 2.20, v2 (obsolete) — Splinter Review
Use HTML::Scrubber + whitelist
Attachment #231268 - Attachment is obsolete: true
Attachment #239147 - Flags: review?(justdave)
Attached patch patch for 2.22, v2.1 (obsolete) — Splinter Review
I forgot to include checksetup.pl in the patch for 2.22.
Attachment #239144 - Attachment is obsolete: true
Attachment #239149 - Flags: review?(justdave)
Attachment #239144 - Flags: review?(justdave)
Attached patch patch for 2.18, v3 (obsolete) — Splinter Review
Use a whitelist instead of a blacklist.
Attachment #238049 - Attachment is obsolete: true
Attachment #239150 - Flags: review?(justdave)
Comment on attachment 239137 [details] [diff] [review]
patch for tip, v5

OK, when I tried to run this on landfil, I got:

undef error - Can't locate auto/HTML/Parser/utf8_mode.al in @INC (@INC contains: . /usr/lib/perl5/5.8.5/i386-linux-thread-multi /usr/lib/perl5/5.8.5
...

After a bit of research, I found this:

------8<------
2004-11-23   Gisle Aas <gisle@ActiveState.com>

     Release 3.39_92
     [...]
     New boolean option; $p->utf8_mode to allow parsing of raw  UTF-8.
------8<------

Landfill has 3.35

Current version is 3.55.

So it looks like we need to require HTML::Parser 3.39_92 or higher (or just call it 3.40 to make it easy)

Unfortunately rpmforge is shipping 3.35 (that's where landfill got it)

With that little nuisance out of the way, it works as advertised.  My one concern was if someone did something sneaky like &#58; (a :) it would think it was a local relative url and let it by...

I tried this:
This is a <a href="javascript&#58;alert('foo')">link</a>.

But it successfully scrubbed it, so HTML::Scrubber is smart. :)

So consider this r+ with the HTML::Parser version check added.
Attachment #239137 - Flags: review?(justdave) → review+
(In reply to comment #84)
> So consider this r+ with the HTML::Parser version check added.

Actually, with it being optional, that makes this tricky... how do we want to handle that?  maybe check the HTML::Parser version before calling utf8_mode()?  Does it break if we don't call it and so the older version is buggy without it?
Attached patch patch for tip, v5.1 (obsolete) — Splinter Review
Make sure that HTML::Parser 3.40 is installed if running Perl 5.8+.
Attachment #239137 - Attachment is obsolete: true
Attachment #239524 - Flags: review?(justdave)
Attachment #239524 - Flags: review?(justdave) → review+
Attached patch patch for 2.22, v2.2 (obsolete) — Splinter Review
Attachment #239149 - Attachment is obsolete: true
Attachment #239830 - Flags: review?(justdave)
Attachment #239149 - Flags: review?(justdave)
grr... if + use = bad idea, even within an eval. I have to require HTML::Parser instead of using it, else Bugzilla crashes if the package has a too low version. I will have to fix the previous patches too.
Attachment #239147 - Attachment is obsolete: true
Attachment #239838 - Flags: review?(justdave)
Attachment #239147 - Flags: review?(justdave)
Comment on attachment 239830 [details] [diff] [review]
patch for 2.22, v2.2

r+ based on interdiff from the previous patch for 2.22
Attachment #239830 - Flags: review?(justdave) → review+
Comment on attachment 239838 [details] [diff] [review]
patch for 2.20, v2.1

r+ based on interdiff from the previous 2.20 patch
Attachment #239838 - Flags: review?(justdave) → review+
Attachment #239830 - Attachment is obsolete: true
Attachment #239839 - Flags: review?(justdave)
Comment on attachment 239839 [details] [diff] [review]
patch for 2.22, v2.3

oh yeah, the "use" thing we already talked about. :)
Attachment #239839 - Flags: review?(justdave) → review+
Attachment #239524 - Attachment is obsolete: true
Attachment #239840 - Flags: review?(justdave)
Comment on attachment 239840 [details] [diff] [review]
patch for tip, v5.2

yup, same as the other one.
Attachment #239840 - Flags: review?(justdave) → review+
Attachment #239150 - Attachment is obsolete: true
Attachment #239843 - Flags: review?(justdave)
Attachment #239150 - Flags: review?(justdave)
Comment on attachment 239843 [details] [diff] [review]
patch for 2.18, v3.1

Had to take a little closer look at this one since we skipped 2.18 in the last round or two of patch updates.  Looks good though.
Attachment #239843 - Flags: review?(justdave) → review+
yay!!
Flags: approval?
Flags: approval2.22?
Flags: approval2.20?
Flags: approval2.18?
Whiteboard: [wanted for 2.18.6] [wanted for 2.20.3][wanted for 2.22.1] patches awaiting review from justdave → ready for 2.18.6] [ready for 2.20.3][ready for 2.22.1][ready for 2.23.3]
As far as I know, the earliest version of Bugzilla affected by this vulnerability is 2.0. 2.10 is the lowest that the Version field allows me to go, though. :-)
Version: 2.17.4 → 2.10
Flags: approval?
Flags: approval2.22?
Flags: approval2.22+
Flags: approval2.20?
Flags: approval2.20+
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+
tip:

Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.54; previous revision: 1.53
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.64; previous revision: 1.63
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.55; previous revision: 1.54
done
Checking in Bugzilla/Install/Requirements.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/Requirements.pm,v  <--  Requirements.pm
new revision: 1.21; previous revision: 1.20
done
Checking in skins/standard/editusers.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/editusers.css,v  <--  editusers.css
new revision: 1.2; previous revision: 1.1
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.26; previous revision: 1.25
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.79; previous revision: 1.78
done
Checking in template/en/default/account/prefs/permissions.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/permissions.html.tmpl,v  <--  permissions.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/account/prefs/settings.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/settings.html.tmpl,v  <--  settings.html.tmpl
new revision: 1.4; previous revision: 1.3
done
Checking in template/en/default/admin/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/classifications/del.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/del.html.tmpl,v  <--  del.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/admin/classifications/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/admin/classifications/reclassify.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl,v  <--  reclassify.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/admin/classifications/select.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl,v  <--  select.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/admin/components/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/admin/components/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/updated.html.tmpl,v  <--  updated.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/admin/groups/delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/delete.html.tmpl,v  <--  delete.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/admin/groups/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/admin/groups/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.8; previous revision: 1.7
done
Checking in template/en/default/admin/keywords/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/keywords/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.9; previous revision: 1.8
done
Checking in template/en/default/admin/products/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/admin/products/edit-common.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit-common.html.tmpl,v  <--  edit-common.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/admin/products/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.6; previous revision: 1.5
done
Checking in template/en/default/admin/products/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/updated.html.tmpl,v  <--  updated.html.tmpl
new revision: 1.4; previous revision: 1.3
done
Checking in template/en/default/admin/settings/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/settings/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/admin/users/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.5; previous revision: 1.4
done
Checking in template/en/default/admin/users/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.3; previous revision: 1.2
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.85; previous revision: 1.84
done
Checking in template/en/default/bug/show-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v  <--  show-multiple.html.tmpl
new revision: 1.33; previous revision: 1.32
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.67; previous revision: 1.66
done
Checking in template/en/default/global/choose-classification.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-classification.html.tmpl,v  <--  choose-classification.html.tmpl
new revision: 1.7; previous revision: 1.6
done
Checking in template/en/default/global/choose-product.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-product.html.tmpl,v  <--  choose-product.html.tmpl
new revision: 1.15; previous revision: 1.14
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.39; previous revision: 1.38
done
Checking in template/en/default/list/list-simple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list-simple.html.tmpl,v  <--  list-simple.html.tmpl
new revision: 1.10; previous revision: 1.9
done
Checking in template/en/default/reports/components.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v  <--  components.html.tmpl
new revision: 1.12; previous revision: 1.11
done
Checking in template/en/default/reports/keywords.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/keywords.html.tmpl,v  <--  keywords.html.tmpl
new revision: 1.9; previous revision: 1.8
done


2.22:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.469.2.15; previous revision: 1.469.2.14
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/Attic/globals.pl,v  <--  globals.pl
new revision: 1.348.2.5; previous revision: 1.348.2.4
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.34.2.1; previous revision: 1.34
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.41.2.3; previous revision: 1.41.2.2
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.45.2.5; previous revision: 1.45.2.4
done
Checking in skins/standard/editusers.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/editusers.css,v  <--  editusers.css
new revision: 1.1.6.1; previous revision: 1.1
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.21.2.2; previous revision: 1.21.2.1
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.61.2.1; previous revision: 1.61
done
Checking in template/en/default/account/prefs/permissions.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/permissions.html.tmpl,v  <--  permissions.html.tmpl
new revision: 1.8.2.1; previous revision: 1.8
done
Checking in template/en/default/account/prefs/settings.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/settings.html.tmpl,v  <--  settings.html.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Checking in template/en/default/admin/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.6.2.2; previous revision: 1.6.2.1
done
Checking in template/en/default/admin/classifications/del.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/del.html.tmpl,v  <--  del.html.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Checking in template/en/default/admin/classifications/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done
Checking in template/en/default/admin/classifications/reclassify.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl,v  <--  reclassify.html.tmpl
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in template/en/default/admin/classifications/select.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl,v  <--  select.html.tmpl
new revision: 1.5.2.1; previous revision: 1.5
done
Checking in template/en/default/admin/components/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in template/en/default/admin/components/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/updated.html.tmpl,v  <--  updated.html.tmpl
new revision: 1.2.4.1; previous revision: 1.2
done
Checking in template/en/default/admin/groups/delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/delete.html.tmpl,v  <--  delete.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done
Checking in template/en/default/admin/groups/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.5.6.1; previous revision: 1.5
done
Checking in template/en/default/admin/groups/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.4.2.3; previous revision: 1.4.2.2
done
Checking in template/en/default/admin/keywords/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/keywords/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.7.2.1; previous revision: 1.7
done
Checking in template/en/default/admin/products/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Checking in template/en/default/admin/products/edit-common.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit-common.html.tmpl,v  <--  edit-common.html.tmpl
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in template/en/default/admin/products/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.5.2.1; previous revision: 1.5
done
Checking in template/en/default/admin/products/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/updated.html.tmpl,v  <--  updated.html.tmpl
new revision: 1.2.2.2; previous revision: 1.2.2.1
done
Checking in template/en/default/admin/settings/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/settings/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.3.4.1; previous revision: 1.3
done
Checking in template/en/default/admin/users/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.2.2.1; previous revision: 1.2
done
Checking in template/en/default/admin/users/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.1.6.1; previous revision: 1.1
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.69.2.3; previous revision: 1.69.2.2
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.54.2.3; previous revision: 1.54.2.2
done
Checking in template/en/default/global/choose-classification.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-classification.html.tmpl,v  <--  choose-classification.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done
Checking in template/en/default/global/choose-product.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-product.html.tmpl,v  <--  choose-product.html.tmpl
new revision: 1.13.2.1; previous revision: 1.13
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.31.2.3; previous revision: 1.31.2.2
done
Checking in template/en/default/list/list-simple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list-simple.html.tmpl,v  <--  list-simple.html.tmpl
new revision: 1.9.8.1; previous revision: 1.9
done
Checking in template/en/default/reports/components.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v  <--  components.html.tmpl
new revision: 1.9.4.1; previous revision: 1.9
done
Checking in template/en/default/reports/keywords.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/keywords.html.tmpl,v  <--  keywords.html.tmpl
new revision: 1.6.12.1; previous revision: 1.6
done


2.20.2:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.412.2.25; previous revision: 1.412.2.24
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.85.2.9; previous revision: 1.85.2.8
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/Attic/globals.pl,v  <--  globals.pl
new revision: 1.326.2.6; previous revision: 1.326.2.5
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.25.2.3; previous revision: 1.25.2.2
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.26.2.5; previous revision: 1.26.2.4
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.28.2.5; previous revision: 1.28.2.4
done
Checking in skins/standard/editusers.css;
/cvsroot/mozilla/webtools/bugzilla/skins/standard/editusers.css,v  <--  editusers.css
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.17.6.4; previous revision: 1.17.6.3
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.43.2.5; previous revision: 1.43.2.4
done
Checking in template/en/default/account/prefs/permissions.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/permissions.html.tmpl,v  <--  permissions.html.tmpl
new revision: 1.6.10.1; previous revision: 1.6
done
Checking in template/en/default/account/prefs/settings.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/settings.html.tmpl,v  <--  settings.html.tmpl
new revision: 1.1.4.2; previous revision: 1.1.4.1
done
Checking in template/en/default/admin/table.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/table.html.tmpl,v  <--  table.html.tmpl
new revision: 1.5.2.1; previous revision: 1.5
done
Checking in template/en/default/admin/classifications/del.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/del.html.tmpl,v  <--  del.html.tmpl
new revision: 1.2.6.1; previous revision: 1.2
done
Checking in template/en/default/admin/classifications/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.3.6.2; previous revision: 1.3.6.1
done
Checking in template/en/default/admin/classifications/reclassify.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/reclassify.html.tmpl,v  <--  reclassify.html.tmpl
new revision: 1.2.6.1; previous revision: 1.2
done
Checking in template/en/default/admin/classifications/select.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/classifications/select.html.tmpl,v  <--  select.html.tmpl
new revision: 1.3.6.1; previous revision: 1.3
done
Checking in template/en/default/admin/components/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Checking in template/en/default/admin/components/updated.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/components/updated.html.tmpl,v  <--  updated.html.tmpl
new revision: 1.2.2.1; previous revision: 1.2
done
Checking in template/en/default/admin/groups/delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/delete.html.tmpl,v  <--  delete.html.tmpl
new revision: 1.4.4.2; previous revision: 1.4.4.1
done
Checking in template/en/default/admin/groups/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.5.4.1; previous revision: 1.5
done
Checking in template/en/default/admin/groups/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/groups/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.1.8.3; previous revision: 1.1.8.2
done
Checking in template/en/default/admin/keywords/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/keywords/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.6.8.1; previous revision: 1.6
done
Checking in template/en/default/admin/products/confirm-delete.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/products/confirm-delete.html.tmpl,v  <--  confirm-delete.html.tmpl
new revision: 1.1.2.2; previous revision: 1.1.2.1
done
Checking in template/en/default/admin/settings/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/settings/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.3.2.1; previous revision: 1.3
done
Checking in template/en/default/admin/users/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in template/en/default/admin/users/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/users/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.1.4.1; previous revision: 1.1
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.60.2.7; previous revision: 1.60.2.6
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.51.2.4; previous revision: 1.51.2.3
done
Checking in template/en/default/global/choose-classification.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-classification.html.tmpl,v  <--  choose-classification.html.tmpl
new revision: 1.4.4.2; previous revision: 1.4.4.1
done
Checking in template/en/default/global/choose-product.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-product.html.tmpl,v  <--  choose-product.html.tmpl
new revision: 1.12.4.1; previous revision: 1.12
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.26.2.5; previous revision: 1.26.2.4
done
Checking in template/en/default/list/list-simple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list-simple.html.tmpl,v  <--  list-simple.html.tmpl
new revision: 1.9.6.1; previous revision: 1.9
done
Checking in template/en/default/reports/components.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v  <--  components.html.tmpl
new revision: 1.9.2.1; previous revision: 1.9
done
Checking in template/en/default/reports/keywords.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/keywords.html.tmpl,v  <--  keywords.html.tmpl
new revision: 1.6.10.1; previous revision: 1.6
done


2.18.5:

Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.289.2.36; previous revision: 1.289.2.35
done
Checking in editcomponents.cgi;
/cvsroot/mozilla/webtools/bugzilla/editcomponents.cgi,v  <--  editcomponents.cgi
new revision: 1.41.2.4; previous revision: 1.41.2.3
done
Checking in editgroups.cgi;
/cvsroot/mozilla/webtools/bugzilla/editgroups.cgi,v  <--  editgroups.cgi
new revision: 1.38.2.3; previous revision: 1.38.2.2
done
Checking in editmilestones.cgi;
/cvsroot/mozilla/webtools/bugzilla/editmilestones.cgi,v  <--  editmilestones.cgi
new revision: 1.23.2.3; previous revision: 1.23.2.2
done
Checking in editproducts.cgi;
/cvsroot/mozilla/webtools/bugzilla/editproducts.cgi,v  <--  editproducts.cgi
new revision: 1.53.2.7; previous revision: 1.53.2.6
done
Checking in editusers.cgi;
/cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v  <--  editusers.cgi
new revision: 1.61.2.9; previous revision: 1.61.2.8
done
Checking in editversions.cgi;
/cvsroot/mozilla/webtools/bugzilla/editversions.cgi,v  <--  editversions.cgi
new revision: 1.22.2.2; previous revision: 1.22.2.1
done
Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/Attic/globals.pl,v  <--  globals.pl
new revision: 1.270.2.13; previous revision: 1.270.2.12
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.10.2.3; previous revision: 1.10.2.2
done
Checking in Bugzilla/Template.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v  <--  Template.pm
new revision: 1.18.2.2; previous revision: 1.18.2.1
done
Checking in Bugzilla/Util.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Util.pm,v  <--  Util.pm
new revision: 1.12.2.6; previous revision: 1.12.2.5
done
Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.12.2.4; previous revision: 1.12.2.3
done
Checking in template/en/default/filterexceptions.pl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v  <--  filterexceptions.pl
new revision: 1.16.2.3; previous revision: 1.16.2.2
done
Checking in template/en/default/account/prefs/permissions.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/account/prefs/permissions.html.tmpl,v  <--  permissions.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done
Checking in template/en/default/admin/keywords/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/admin/keywords/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.4.2.1; previous revision: 1.4
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.40.2.7; previous revision: 1.40.2.6
done
Checking in template/en/default/bug/create/create.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/create/create.html.tmpl,v  <--  create.html.tmpl
new revision: 1.30.2.4; previous revision: 1.30.2.3
done
Checking in template/en/default/global/choose-product.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/choose-product.html.tmpl,v  <--  choose-product.html.tmpl
new revision: 1.10.2.2; previous revision: 1.10.2.1
done
Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.17.2.6; previous revision: 1.17.2.5
done
Checking in template/en/default/list/list-simple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/list-simple.html.tmpl,v  <--  list-simple.html.tmpl
new revision: 1.7.2.2; previous revision: 1.7.2.1
done
Checking in template/en/default/reports/components.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/components.html.tmpl,v  <--  components.html.tmpl
new revision: 1.8.2.1; previous revision: 1.8
done
Checking in template/en/default/reports/keywords.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/reports/keywords.html.tmpl,v  <--  keywords.html.tmpl
new revision: 1.6.2.1; previous revision: 1.6
done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Summary: Fix escaping/quoting in editXXX.cgi scripts → [SECURITY] Fix escaping/quoting in editXXX.cgi scripts
Security Advisory has been sent, so this bug is no longer private.
Group: webtools-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: