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

[Raphaël Hertzog]

Looking at ​https://github.com/django/django/commit/4f6fffc1dc429f1ad428ecf8e6620739e8837450 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).

[Raphaël Hertzog]

Suggested patch

[Carl Meyer]

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 ​https://github.com/django/django/commit/681550ca6d5ff3e591a769643e37fd0b50f29c91, 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.

[Carl Meyer]

For historical interest - the custom copy of get_environ() was introduced in the earliest days of Django (in ​https://github.com/django/django/commit/b68c478a), 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.

[Carl Meyer]

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.

[Tim Graham]

Removed the copied method as suggested in #24238.