request forgery check displayed when only viewing admin pages

Bug #1160647 reported by Phil Sutter
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
GNU Mailman
Fix Released
Low
Mark Sapiro

Bug Description

CSRf checking in admin.py is buggy. Logging into the admin interface succeeds fine, but when clicking any of the links on the page top, the request forgery error message is displayed on the resulting page.

The problem is basically that Cgi/admin.py is called with only a single param in cgidata, namely 'admin' (which is empty). Since this param is not part of the safe_params list, csrf_check() is called with 'None' as second parameter.

Since submitting forms is working fine, this bug is merely a cosmetic one but still very confusing.

Related branches

Revision history for this message
Mark Sapiro (msapiro) wrote :

It works for me. What Mailman version is this? How was it installed. What browser are you using? What do the actual Links look like?

In my case, a URL like http://www.example.com/mailman/admin/list/privacy/sender or http://www.example.com/mailman/admin/list/passwords produces a FieldStorage instance which is empty, i.e. cgidata.keys() is an empty list.

Changed in mailman:
assignee: nobody → Mark Sapiro (msapiro)
status: New → Incomplete
Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

This is with version 2.1.15 on funtoo linux. Installed via portage (the local package manager), but synchronised /var/lib/mailman from an older installation. Cgi is served via lighttpd and accessed using firefox 19.0.2. LInks look like e.g. '<a href="../../admin/evtf/general"><strong>[Allgemeine Optionen]</strong></a>', so I am not quite sure about where the 'admin' param comes from. Maybe lighttpd does this, my lighttpd.conf contains this:
        alias.url = (
                "/mailman-icons" => "/usr/lib64/mailman/icons/",
                "/pipermail" => "/var/lib/mailman/archives/public/",
                "/" => "/usr/lib64/mailman/cgi-bin/",
        )
        cgi.assign = (
                "/admin" => "",
                "/admindb" => "",
                "/confirm" => "",
                "/create" => "",
                "/edithtml" => "",
                "/listinfo" => "",
                "/options" => "",
                "/private" => "",
                "/rmlist" => "",
                "/roster" => "",
                "/subscribe" => ""
        )

Revision history for this message
Mark Sapiro (msapiro) wrote :

I know nothing about lighttpd configuration, but it looks to me like

        cgi.assign = (
                "/admin" => "",
                "/admindb" => "",
                "/confirm" => "",
                "/create" => "",
                "/edithtml" => "",
                "/listinfo" => "",
                "/options" => "",
                "/private" => "",
                "/rmlist" => "",
                "/roster" => "",
                "/subscribe" => ""
        )

is exactly the problem. Is that necessary? What happens if you remove it?

Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

Sadly not. Removing this assignment makes lighttpd handle these files statically, i.e. deliver them instead of executing them.

For testing, I have replaced /usr/lib/mailman/cgi-bin/admin with some wrapper script which prints it's arguments, the environment and stdin first before executing the real 'admin'. Strangely, there's nothing suspicous there. Despite digging through the mailman and python cgi code for a while now, I have no idea where the added parameter comes from.

Do you have any idea where to continue searching for the bug's source?

Revision history for this message
Mark Sapiro (msapiro) wrote :

What exactly do you get if you save the attached 'printenv' to /usr/lib/mailman/cgi-bin/ and go to the appropriate URL?

You probably have to add "/printenv" => "", to lighttpd's cgi.assign, or you could just temporarily replace a lesser used script like create or edithtml with this one.

Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

I get this output:

REDIRECT_STATUS: 200
SERVER_SOFTWARE: lighttpd/1.4.32
SCRIPT_NAME: /printenv
REQUEST_METHOD: GET
SERVER_PROTOCOL: HTTP/1.1
CONTENT_LENGTH: 0
HTTP_USER_AGENT: Mozilla/5.0 (X11; Linux x86_64; rv:19.0) Gecko/20100101 Firefox/19.0
HTTP_CONNECTION: keep-alive
SERVER_NAME: lists.nwl.cc
REMOTE_PORT: 32968
SERVER_PORT: 80
SERVER_ADDR: ::
DOCUMENT_ROOT: /usr/lib64/mailman/cgi-bin/
SCRIPT_FILENAME: /usr/lib64/mailman/cgi-bin/printenv
HTTP_DNT: 1
HTTP_HOST: lists.nwl.cc
REQUEST_URI: /printenv
HTTP_ACCEPT: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
GATEWAY_INTERFACE: CGI/1.1
REMOTE_ADDR: ::ffff:95.89.182.210
HTTP_ACCEPT_LANGUAGE: de-de,de;q=0.8,en-us;q=0.5,en;q=0.3
HTTP_ACCEPT_ENCODING: gzip, deflate

Stdin:

CGI:
FieldStorage(None, None, [])

Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

Note that this is expected behaviour. After logging in for the admin interface, no error is reported. The request forgery check message appears only after having clicked one of the links in the page's top. I should probably try to insert debugging output somewhere there, searching for any differences.

Revision history for this message
Mark Sapiro (msapiro) wrote :

Aaah... I missed that. Look at the source of the page in your browser. How do the hrefs differ from what you use to go there. They will probably be relative, and I'n not interested in that difference, but if you absolutize them and then go directly, do you see the problem? If it has to do with the url being admin/LIST/general rather than admin/LIST, what to you get from the rrintenv script if you invoke it with printenv/aaaa/bbbb?

I suspect that the issue may be related to your alias_url "/" => "/usr/lib64/mailman/cgi-bin/" what if you insert "/admin" => "/usr/lib64/mailman/cgi-bin/admin" ?

Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

These two tests do not make any difference to the symptom. Though I think I found the culprit, but first things first.

The reason why no error message is displayed right after login is that the login process is itself a form submission, and those are working as initially stated. So the problem is not really "caused" by clicking one of the page top links, also indicated by a browser page refresh also showing the error message.

I did in turn add the same debug output to admin.py itself, which did not show anything special other than that the FieldStorage constructor returned 'FieldStorage(None, None, [MiniFieldStorage('admin', '')])'. While looking at the class's implementation again, I noticed that sys.argv is parsed while constructing the internal list so I added 'sys.argv[1]' to my debugging statement -> bingo!

This is what happens:
- the cgi-wrapper 'cgi-bin/admin' calls 'scripts/driver' with the scriptname as first parameter (i.e. 'admin')
- 'scripts/driver' then uses 'sys.argv[1]' to find out it has to call 'main()' in 'Cgi/admin.py', but leaves 'sys.argv' intact
- 'Cgi/admin.py' then calls the FieldStorage constructor which finds data in 'sys.argv[1]'
(the FieldStorage class blows my mind, so I didn't bother digging to the point where this actually turns into a request parameter value)

Could this be a problem with python2.7 (default on my system) instead of python3? Since the 'printenv' code you suggested is also python2 syntax I guess this should be a working configuration, no?

Just for the record: in 'scripts/driver', below line 94 which reads 'scriptname = sys.argv[1]' I added a new line: 'sys.argv[1] = ""' which indeed made the error message disappear. Didn't test full functionality though.

Revision history for this message
Mark Sapiro (msapiro) wrote : Re: [Bug 1160647] Re: request forgery check displayed when only viewingadmin pages

>The reason why no error message is displayed right after login is that
>the login process is itself a form submission, and those are working as
>initially stated. So the problem is not really "caused" by clicking one
>of the page top links, also indicated by a browser page refresh also
>showing the error message.

OK, I didn't understand that the problem only didn't occur when coming
from the login screen. I thought you were OK on the direct URL visit
even if already logged in. But I guess not.

>This is what happens:
>- the cgi-wrapper 'cgi-bin/admin' calls 'scripts/driver' with the scriptname as first parameter (i.e. 'admin')
>- 'scripts/driver' then uses 'sys.argv[1]' to find out it has to call 'main()' in 'Cgi/admin.py', but leaves 'sys.argv' intact
>- 'Cgi/admin.py' then calls the FieldStorage constructor which finds data in 'sys.argv[1]'
>(the FieldStorage class blows my mind, so I didn't bother digging to the point where this actually turns into a request parameter value)
>
>Could this be a problem with python2.7 (default on my system) instead of
>python3? Since the 'printenv' code you suggested is also python2 syntax
>I guess this should be a working configuration, no?

AFIK, Mailman 2.1 has not been well tested if at all with Python 2.7.
It definitely will not work with Python 3.x. This CGI has been well
exercised with Pythons 2.1 through 2.6,

I will investigate the possibility of a Python 2.7 issue. It's time I
upgraded my test platforms and production server anyway ;)

>Just for the record: in 'scripts/driver', below line 94 which reads
>'scriptname = sys.argv[1]' I added a new line: 'sys.argv[1] = ""' which
>indeed made the error message disappear. Didn't test full functionality
>though.

It should be OK if it works. Even better might be

        del sys.argv[1]

--
Mark Sapiro <email address hidden> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan

Revision history for this message
Mark Sapiro (msapiro) wrote :

> I will investigate the possibility of a Python 2.7 issue. It's time I
> upgraded my test platforms and production server anyway ;)

I'm running Mailman now with Python 2.7.4 and not seeing this issue.

--
Mark Sapiro <email address hidden> The highway is for gamblers,
San Francisco Bay Area, California better use your sense - B. Dylan

Revision history for this message
Mark Sapiro (msapiro) wrote :

OK. I finally looked at the code in Python's cgi module. It adds sys.argv[1:] to the list of query parameters for a GET if and only if there is no QUERY_STRING in the environment. Apache provides an empty QUERY_STRING when the URL doesn't have one and lighttpd does not.

This is a bug in lighttpd. RFC 3875 says:

   The server MUST set this variable; if the Script-URI does not include
   a query component, the QUERY_STRING MUST be defined as an empty
   string ("").

I will consider defending against this bug by making scripts/driver drop all but the first item in sys.argv, but it is really a lighttpd bug, not a Mailman bug.

Thanks for your help in identifying the underlying issue.

Mark Sapiro (msapiro)
Changed in mailman:
importance: Undecided → Low
milestone: none → 2.1.16
status: Incomplete → Fix Committed
Revision history for this message
Phil Sutter (p-launchpad-q) wrote :

Oh well. Quick google search turned up this: http://redmine.lighttpd.net/issues/1339
so, this is actually a known issue in lighttpd for over two years now.

Mark, thanks a lot for your time and effort in analysing this problem. Not sure if I ever would have found this on my own. I will address lighttpd's maintainers regarding this issue, hopefully this will eventually get a real fix.

Mark Sapiro (msapiro)
Changed in mailman:
milestone: 2.1.16 → 2.1.16rc1
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.