319087 - reports.cgi throws an error if you pass in an invalid "datasets" parameter

Status: RESOLVED FIXED

(Bugzilla :: Reporting/Charting, defect)

Description

[Max Kanat-Alexander]

From Tim Brown <[e-mail]>:

Bugzilla 2.18.4, 2.20

The path to the Bugzilla installation is disclosed when a request is made to reports.cgi with an invalid datasets parameter which fails the regular expression m/^[A-Za-z0-9:]+$/.

Comment 1

[Frédéric Buclin]

not a security bug

Comment 2

[Tim Brown]

(In reply to comment #1)
> not a security bug
> 

No? http://www.osvdb.org/searchdb.php?action=search_title&vuln_title=path+disclosure&Search=Search

Comment 3

[Frédéric Buclin]

bah... fixing this bug in a clean way is not that easy. I detected two places where the path is displayed.

- the first one is trivial to fix, at line 236:

    if ($datasets !~ m/^[A-Za-z0-9:]+$/) {
        die "Invalid datasets $datasets";
    }

We simply have to replace die() by ThrowUserError().

- the second one is bothering me, at line 345:

    $img->set (%settings);
    $img->$type($image_file, [ @data{('DATE', @labels)} ]);

$img->$type() fails and throws an error (using croak(), i.e. with the path in it) if you pass an invalid datasets value. As the exact reason of the failure is internal to Chart::Lines (or more precisely to Chart::Base), we cannot hardcore the reason of the failure in code-error.html.tmpl. And using eval {} and $@ would require to remove the path from it, which cannot easily be done as:

        my $path = cwd();
        my $error = $@;
        $error =~ s/\Q$path\E//g;

because some installations, such as mines, use simlinks to point to the right directory, and so the regexp will fail to remove the path and the path would still be exposed. In this case, I see no other choice than hidding the exact reason, but this makes debugging much harder.

Comment 4

[Frédéric Buclin]

(In reply to comment #3)
> we cannot
> hardcore the reason of the failure in code-error.html.tmpl. And using eval {}
> and $@ would require to remove the path from it, which cannot easily be done

> In this case, I see no other choice than hidding the exact
> reason, but this makes debugging much harder.

What we could do is to show a generic error to unprivileged users ('an internal error occured while generating the graph, please report this error to the admin') and display the exact error if the user is an admin.

Comment 5

[Frédéric Buclin]

The 2.22 branch is restricted to security bugs and this bug is not critical enough -> 3.0.

Comment 6

[Frédéric Buclin]

I fixed the issue reported in comment 0 as part of bug 340253.