237627 - reports.cgi doesn't detect invalid dataset name

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Teemu Mannermaa (:wicked)]

There is code in reports.cgi chart_image_name sub that supposedly would yell if
an invalid dataset name is specified. Unfortunately the code uses broken regexp
so it won't detect the condition correctly.

Comment 1

[Teemu Mannermaa (:wicked)]

Attachment: One line change to fix the regexp

This new regexp will now correctly detect if there are other than letters,
numbers or :-characters in any of the dataset names.

Comment 2

[Dave Miller [:justdave]]

Let's hold this for a security advisory.

This can be exploited as an arbitrary filename path or XSS.

Comment 3

[Dave Miller [:justdave]]

Just checked...   2.16 is affected by this, too, so we need a patch for 2.16

Comment 4

[Dave Miller [:justdave]]

hmmm, okay, I can't actually find a way to actually exploit this.  Chart::Base
dies with "invalid symbol reference" if you try to pass in a dataset name that
contains a /, so that eliminates pathname garbage.  And it doesn't appear to
actually display the datasets that it can't actually find except as part of the
image filename, which obviously remains escaped somehow...

<img
src="graphs/-All-_NEW_ASSIGNED_REOPENED_UNCONFIRMED_%3Cscript%3Ealert%28%27foo%27%29.png">

Anyone else have any ananysis they can provide?  We should probably still fix
this in 2.16, but I'm starting to think it's not an exploitable security hole now...

Comment 5

[Teemu Mannermaa (:wicked)]

Tested and it seems same patch will apply to tip of 2.16 branch flawlessly.

Comment 6

[Joel Peshkin]

Attachment: de-bitrotted version specifically for 2.18

While attachment 144032 [details] [diff] [review] is right for 2.16, this is de-bitrotted to apply
smoothly  for 2.18

Comment 7

[Joel Peshkin]

checked in on both branches

Comment 8

[Dave Miller [:justdave]]

Clearing the security flag on disclosed bugs