micropython / micropython-lib

Core Python libraries ported to MicroPython
Other
2.3k stars 980 forks source link

SECURITY: Requests module leaks passwords & usernames for HTTP Basic Auth #839

Open jonfoster opened 3 months ago

jonfoster commented 3 months ago

While looking at the MicroPython requests module (on the git HEAD), I noticed this:

If you make a request with HTTP basic auth (a username/password) and did not specify a headers dict, then I believe the username and password would be added to the default headers to be used for every subsequent HTTP request. Even if that request is to a completely different server, which you don't trust with your username and password. That's probably not a good idea.

I haven't verified this, it's just from reading the code, but someone should probably look into it.

This is because there is headers={} in the function prototype, specifying a default for the headers parameter. But (at least in cPython) that same dictionary will get reused for every call that doesn't explicitly specify a headers parameter. So if the function changes the headers dictionary - such as by adding an Authorization header - that change will be there for every future call of the function. This is a known dangerous part of the Python language, you're not the first people to write this kind of bug.

To fix this, you could keep the auth headers separate from the headers variable. Something like this (totally untested!) commit: https://github.com/jonfoster/micropython-lib/commit/92e9b2208814faa22ba3ff2a19cbc8e0c5210a47 - feel free to use that as a starting point.

Gadgetoid commented 3 weeks ago

The MicroPython way aiui would be to mirror CPython's solution to this problem, which uses a None default value and then sets it to an empty dict at runtime:

https://github.com/psf/requests/blob/0e322af87745eff34caffe4df68456ebc20d9068/src/requests/models.py#L258-L276

And I see this is exactly what #823 does. That'll teach me not to look at PRs first 😆