872263 - [traceback] MemcachedKeyCharacterError: Control characters not allowed

Status: RESOLVED FIXED

(Marketplace Graveyard :: General, defect, P2)

Description

[Rob Hudson [:robhudson]]

http://sentry.dmz.phx1.mozilla.com/addons/marketplace/group/13988/

It looks like in session_csrf the key ends up looking something like this:

"|dir; carrier=telefonica; multidb_pin_writes=y; sessionid="

and memcache doesn't like it, which means it's not going to get cached.

Comment 1

[Wil Clouser [:clouserw]]

Andy: do you know anything about this bug?  It came up with something you did with the API.

These are really weird keys.  If we actually need them we should be hashing them?

Comment 2

[Christopher Van Wiemeersch [:cvan]]

Acunetix-Product	
WVS/8.0 (Acunetix Web Vulnerability Scanner - NORMAL)
Acunetix-Scanning-Agreement	
Third Party Scanning PROHIBITED
Acunetix-User-Agreement	
http://www.acunetix.com/wvs/disc.htm

Comment 3

[Wil Clouser [:clouserw]]

Looks like this is a web vulnerability scanner.  I don't think there is a security issue here but we should fix the crash.  Definitely shouldn't be passing user data into memcache keys.

Comment 4

[Allen Short [:ashort]]

Looks like the literal key is being sent to memcache via django-session-csrf. 
https://github.com/mozilla/django-session-csrf/blob/master/session_csrf/__init__.py#L60

Should this be hashed/base64ed?

Comment 5

[Wil Clouser [:clouserw]]

Well, I don't know that there is a security vulnerability there, but just as a general rule of thumb I would hash or encode it, yeah.  Maybe we should file a bug against django-session-csrf to see if they considered that?

At a minimum we shouldn't crash here (try/catch).

Comment 6

[Allen Short [:ashort]]

https://github.com/mozilla/django-session-csrf/commit/13c3cb212a
https://github.com/mozilla/zamboni/commit/d50a1122