incorrect filter name detection

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Testing Suite
--
minor
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

2.19.3
Bugzilla 2.18
Bug Flags:
approval +
approval2.20 +
approval2.18 +

Details

Attachments

(1 attachment)

(Assignee)

Description

13 years ago
If an invalid filter name is the concatenation of a valid filter name followed
by any string (such as "htmllpsolit", "html" being a valid filter name),
008filter.t does not complain because its regexp is:

    return 1 if $directive =~ /FILTER\ (html|csv|js|url_quote|css_class_quote|
                                        ics|quoteUrls|time|uri|xml|lower|
                                        obsolete|inactive|closed|unitconvert|
                                        none)/x;

meaning that it looks for a substring at the beginning of the filter name only,
as in my example above.

Moreover, this test cannot catch an invalid filter name if this one is mixed
with a valid one, such as "FILTER html FILTER lpsolit".

This may lead to security holes, as unfiltered data may be displayed, see for
instance bug 297845, even if I wonder whether Template.pm would complain or not.

Setting the security flag till bug 297845 is fixed and till we determine if this
is exploitable or not.
(Assignee)

Updated

13 years ago
No longer depends on: 297845
(Assignee)

Comment 1

13 years ago
ok, it's not exploitable. Templates.pm complains if it detects invalid filter
names. Bug 297845 is invalid because my testcase isn't due to a bad filter name
detection but due to the fact that the milestone URL is not filtered in
editproducts.cgi (this file really need to be templatized asap).
Group: webtools-security
Severity: normal → minor

Comment 2

13 years ago
We could probably fix this just by adding a /s+ at the beginning and end of the
regex.
(Assignee)

Comment 3

13 years ago
Created attachment 202357 [details] [diff] [review]
patch, v1

this patch prevents bad filter names such as FILTER htmli, which isn't detected actually.
Assignee: zach → LpSolit
Status: NEW → ASSIGNED
Attachment #202357 - Flags: review?(wurblzap)
Comment on attachment 202357 [details] [diff] [review]
patch, v1

r=wurblzap by inspection.
Attachment #202357 - Flags: review?(wurblzap) → review+
Flags: approval?
Target Milestone: --- → Bugzilla 2.22
(Assignee)

Comment 5

13 years ago
incorrect filtering may have security implications, even if TT should complain that some given filter doesn't exist. The patch applies on 2.18 too.
Flags: approval2.20?
Flags: approval2.18?
Target Milestone: Bugzilla 2.22 → Bugzilla 2.18
I wonder if there's some reason why we did it like we did.  cc:ing gerv, who might know.  This patch certainly looks like the right thing to do.
Flags: approval?
Flags: approval2.20?
Flags: approval2.20+
Flags: approval2.18?
Flags: approval2.18+
Flags: approval+
(Assignee)

Comment 7

13 years ago
tip:

Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.21; previous revision: 1.20
done

2.20:

Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.17.6.2; previous revision: 1.17.6.1
done

2.18.4:

Checking in t/008filter.t;
/cvsroot/mozilla/webtools/bugzilla/t/008filter.t,v  <--  008filter.t
new revision: 1.12.2.2; previous revision: 1.12.2.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
No particularly good reason, if the current patch works :-)

Gerv
You need to log in before you can comment on or make changes to this bug.