1252216 - Push extension configuration is vulnerable to CSRF

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Wladimir Palant]

Push extension's configuration page lacks CSRF protection. This means that somebody can lure Bugzilla admins on a page containing the following code:

> <img src="https://bugzilla-dev.allizom.org/page.cgi?id=push_config.html&save=1&global.enabled=Disabled&global.log_purge=0&Aha.enabled=Disabled&Aha.account_domain=&Aha.account_realm=&Aha.username=&Aha.password=&File.enabled=Enabled&File.filename=../show_bug.cgi&ReviewBoard.enabled=Disabled&ReviewBoard.product=&ReviewBoard.component=&ReviewBoard.version=&ReviewBoard.group=&ReviewBoard.cc=&TCL.enabled=Disabled&TCL.tcl_user=&TCL.sftp_host=&TCL.sftp_port=&TCL.sftp_user=&TCL.sftp_pass=&TCL.sftp_remote_path=">

This will successfully change configuration parameters of the Push extension, and in this particular case disable almost everything. I didn't spend much time to investigate abuse potential here, in particular what the security group setting for ReviewBoard is doing. The link used above demonstrates one issue however: by setting File.filename parameter one can override files, even if they aren't located inside Bugzilla's data directory. I didn't bother figuring out whether one can somehow control the data being written to these files, but it allows adding data to a CGI file meaning that this could turn into a code execution vulnerability.

Comment 1

[David Lawrence [:dkl]]

Attachment: 1252216_1.patch

- Added csrf token checking for configuration UI
- Added relative path check for File.pm filenames

Comment 2

[David Lawrence [:dkl]]

Attachment: 1252216_2.patch

Forgot to call delete_token()

Comment 3

[Dylan Hardison [:dylan] (he/him)]

(In reply to Wladimir Palant from comment #0)
> ... but it allows adding data to a CGI file
> meaning that this could turn into a code execution vulnerability.

It is bad that paths weren't checked, but you'd have to find another location to abuse -- only data is writable by the httpd process. "Could turn into" would require code changes because the permissions are enforced by bugzilla code.

Amusingly, if anyone were to use this extension on Windows, the check would be completely meaningless because paths don't start with /.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   4d95649..e5b9aa6  master -> master

Comment 5

[Wladimir Palant]

(In reply to Dylan William Hardison [:dylan] from comment #3)
> It is bad that paths weren't checked, but you'd have to find another
> location to abuse -- only data is writable by the httpd process. "Could turn
> into" would require code changes because the permissions are enforced by
> bugzilla code.

Sure, I could only judge my test installation where this location looked writable. The other obvious victim would be "params" file of course - it's located inside the data directory and yet being executed (running inside Safe but probably able to do damage regardless). Or the template cache - it's located outside the data directory but I suspect that it must be writable.

Comment 6

[Daniel Veditz [:dveditz]]

dkl: could you take a stab at a security rating on this bug? What could be done to the Push extension, what damage would that cause to the installation?

Comment 8

[Daniel Veditz [:dveditz]]

sec-moderate based on feedback from dkl.