3 ms·

Information Exposure Vulnerability with Django and Memcached

31 points by danpalmer 6y ago

danpalmer 6y ago
The repo demonstrating this is now open source: https://github.com/danpalmer/django-cve-2020-13254 https://github.com/danpalmer/django-cve-2020-13254
toxik 6y ago
I would like to note that libmemcached does expose key validation as a behavior. It’s off by default though, which I think it probably should not be. However I’m not sure about the performance impact in typical workloads — some people make quite large gets.
- danpalmer 6y ago
  Interesting, I hadn't seen that, updated. I don't think `pylibmc` exposes it, or again not by default. I actually don't think that it's wrong for a driver library to have these sorts of features disabled by default, because the target audience is typically a slightly higher level of abstraction rather than the end-developer. I think the aim should be to mimic the API of the service as closely as possible with the same defaults where possible. It's then the responsibility of the abstraction layer closer to the application to decide what's more appropriate on that end, and for that to be resolved between the two.
  - toxik 6y ago
    I think Django should set the behavior by default. It is exposed by pylibmc. I know because I wrote it.
    - danpalmer 6y ago
      Yep I agree. That's pretty much what the vulnerability was about! (Although Django decided to implement the check itself as it can be therefore be implemented in the same way for all Memcached backends)
gsu2 6y ago
Why not force memcached clients to use the binary protocol instead of the ASCII protocol? The length check would still be necessary, but spaces (and other arbitrary bytes) would no longer break things.
- danpalmer 6y ago
  It's not really possible in Django to force this as each library will implement that in a different way (or may not even support the binary protocol). Django adding validation is easy, unlikely to cause issues, and makes all usage safe unless the user explicitly overrides the behaviour. Plus, Django actually implements these checks for non-Memcached backends, raising warnings if keys aren't Memcached compatible. This is done because very often in local development users might run the "local memory" cache backend, which is just a Python dict. In this case they probably want to know that what they're doing won't work when it gets to Memcached in production.
typenil 6y ago
Great work tracking this down and making it so clear with the example repo. Gave me the chance to audit my projects that are lagging behind the Django patch.

31 points by danpalmer 6y ago

The repo demonstrating this is now open source: https://github.com/danpalmer/django-cve-2020-13254 https://github.com/danpalmer/django-cve-2020-13254

toxik 6y ago

I would like to note that libmemcached does expose key validation as a behavior. It’s off by default though, which I think it probably should not be. However I’m not sure about the performance impact in typical workloads — some people make quite large gets.

danpalmer 6y ago
Interesting, I hadn't seen that, updated. I don't think `pylibmc` exposes it, or again not by default. I actually don't think that it's wrong for a driver library to have these sorts of features disabled by default, because the target audience is typically a slightly higher level of abstraction rather than the end-developer. I think the aim should be to mimic the API of the service as closely as possible with the same defaults where possible. It's then the responsibility of the abstraction layer closer to the application to decide what's more appropriate on that end, and for that to be resolved between the two.
- toxik 6y ago
  I think Django should set the behavior by default. It is exposed by pylibmc. I know because I wrote it.
  - danpalmer 6y ago
    Yep I agree. That's pretty much what the vulnerability was about! (Although Django decided to implement the check itself as it can be therefore be implemented in the same way for all Memcached backends)

gsu2 6y ago

Why not force memcached clients to use the binary protocol instead of the ASCII protocol? The length check would still be necessary, but spaces (and other arbitrary bytes) would no longer break things.

danpalmer 6y ago
It's not really possible in Django to force this as each library will implement that in a different way (or may not even support the binary protocol). Django adding validation is easy, unlikely to cause issues, and makes all usage safe unless the user explicitly overrides the behaviour. Plus, Django actually implements these checks for non-Memcached backends, raising warnings if keys aren't Memcached compatible. This is done because very often in local development users might run the "local memory" cache backend, which is just a Python dict. In this case they probably want to know that what they're doing won't work when it gets to Memcached in production.

typenil 6y ago

Great work tracking this down and making it so clear with the example repo. Gave me the chance to audit my projects that are lagging behind the Django patch.