Opened 9 years ago

Closed 9 years ago

#24239 closed Bug (duplicate)

Possible problem in CVE-2015-0219 fix for 1.4.x

Reported by: Raphaël Hertzog Owned by: nobody
Component: HTTP handling Version: 1.4
Severity: Normal Keywords:
Cc: carl@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Looking at you will see that the commit adds a get_environ() method to the WSGIRequestHandler class.

But in fact, the class already has such a method, so this change effectively replaces the initial get_environ() implementation with another one that relies on ancestors providing the bulk of the code while the original implementation does everything alone.

I would have expected this to blow up in some interesting ways... I'm not using 1.4 myself so I haven't verified. I discovered this while trying to backport this patch for an even older version of Django (1.2 in Debian Squeeze).

Attachments (1)

0001-1.4.x-Fixed-24239-merge-both-WSGIRequestHandler.get_.patch (2.3 KB ) - added by Raphaël Hertzog 9 years ago.
Suggested patch

Download all attachments as: .zip

Change History (6)

by Raphaël Hertzog, 9 years ago

Suggested patch

comment:1 by Raphaël Hertzog, 9 years ago

Has patch: set

comment:2 by Carl Meyer, 9 years ago

Component: UncategorizedHTTP handling
Triage Stage: UnreviewedAccepted

Hi - thanks for the report! This was a mistake in backporting that patch to 1.4. However, I'm pretty sure in practice it's harmless - the custom implementation of get_environ() there has been unnecessary for a long time (in fact, I'm not sure why it was ever necessary at all). It was simply removed (without requiring any compensating changes) in, which is why it doesn't exist in more recent Django versions. Relying on the standard-library get_environ() implementation is better.

If this issue had been caught before the release, I would say your fix is the right one, for stability reasons. But now that we have a 1.4 release in the wild which effectively is using the stdlib's get_environ() anyway (and nobody has reported a practical problem with that, nor would I expect any), I think we should just remove the custom get_environ() in the 1.4.x branch.

comment:3 by Carl Meyer, 9 years ago

For historical interest - the custom copy of get_environ() was introduced in the earliest days of Django (in, in the initial commit that created runserver. There was no explanation in that commit of why the method was copied. I suppose (again, just for historical interest) one could compare it with the get_environ() method present in whatever stdlib was current in July 2005 to see what, if any, changes Adrian made to it.

comment:4 by Carl Meyer, 9 years ago

Actually, scratch that, the reason is clear. In July 2005, wsgiref was a third-party library, it wasn't in the stdlib yet. So Adrian was just copying code from it - at that time Django's WSGIRequestHandler inherited from BaseHTTPHandler, which didn't have a get_environ(), of course. So the custom get_environ() method never existed in order to correct any issue in wsgiref - it was a straight-up copy and has been superfluous ever since Django's WSGIRequestHandler began inheriting from wsgiref.simple_server.WSGIRequestHandler.

comment:5 by Tim Graham, 9 years ago

Resolution: duplicate
Status: newclosed

Removed the copied method as suggested in #24238.

Note: See TracTickets for help on using tickets.
Back to Top