`parse_cookie_str` can be crashed by end users via `cookie_str = 'true'`

[gmseb]

Hi!

I stumbled upon a crash report ValueError: not enough values to unpack (expected 2, got 1) in Sentry from django-cookie-consent code line…

https://github.com/jazzband/django-cookie-consent/blob/81aae63a289d67cdc9803c61ae6d3239a2af83a2/cookie_consent/util.py#L15

…today. cookie_str has value "true" in this case, and playing with IPython shows how the current code breaks:

In [1]: 'foo=bar'.split('=')
Out[1]: ['foo', 'bar']

In [2]: ''.split('=')
Out[2]: ['']

In [3]: 'true'.split('=')
Out[3]: ['true']

The issue still exists on master so until this is fixed, Django setups can be crashed like this. Would be great to have fixed, thank you!

Best, Sebastian

[some1ataplace]

The current implementation assumes every part of the cookie string will have a key-value pair separated by "=", but your test assumes that this is not always the case. Curious how and why you are parsing your cookie string differently. If you can shed some insight or your particular use case it would be helpful.

Perhaps something like this could suffice:

def parse_cookie_str(cookie):
    dic = {}
    if not cookie:
        return dic
    
    for c in cookie.split("|"):
        # Only split on the first "=" to handle cases with "=" in the value
        parts = c.split("=", 1)
        
        # Handle different split scenarios
        if len(parts) == 2:
            key, value = parts
            dic[key] = value
        elif len(parts) == 1:
            # If no "=" is found, skip this part
            continue
    
    return dic

# Test Scenarios
scenarios = [
    # Normal case
    "Ads=1|reCAPTCHA=1|Stripe=1|Youtube=1",
    
    # Empty string
    "",
    
    # True
    "true",
    
    # Single item
    "Ads=1",
    
    # Malformed item
    "Ads=1|reCAPTCHA|Stripe=1",
    
    # Value with '=' 
    "Ads=google=tracking|Stripe=1",
    
    # Multiple '=' in value
    "Tracking=google==analytics|Ads=1"
]

for scenario in scenarios:
    print(f"\nScenario: '{scenario}'")
    result = parse_cookie_str(scenario)
    print("Result:", result)

Output:

Scenario: 'Ads=1|reCAPTCHA=1|Stripe=1|Youtube=1'
Result: {'Ads': '1', 'reCAPTCHA': '1', 'Stripe': '1', 'Youtube': '1'}

Scenario: ''
Result: {}

Scenario: 'true'
Result: {}

Scenario: 'Ads=1'
Result: {'Ads': '1'}

Scenario: 'Ads=1|reCAPTCHA|Stripe=1'
Result: {'Ads': '1', 'Stripe': '1'}

Scenario: 'Ads=google=tracking|Stripe=1'
Result: {'Ads': 'google=tracking', 'Stripe': '1'}

Scenario: 'Tracking=google==analytics|Ads=1'
Result: {'Tracking': 'google==analytics', 'Ads': '1'}

[tomcheney]

I am also seeing this issue in Sentry not enough values to unpack (expected 2, got 1) in the field but cannot see why the cookie value would be in the incorrect format. Possibly not a real user? I think just adding some robustness as suggested by @some1ataplace would be an appropriate solution.

[sergei-maertens]

Thanks for the report - I'll see if I have some time during the djangocon sprints today to work on some robustness.

[sergei-maertens]

The only way I can imagine this happens if the cookie is not set to HttpOnly and some client-side Javascript is incorrectly setting cookie values.

The util function dict_to_cookie_str seems to be the only source which writes to our cookie server side, and that takes care of only putting in = separated key-value pairs. There is a risk the value contains = which will break things in other ways though.

So, I would attribute this to either faulty JS that's setting invalid cookie values, or indeed, some kind of bot/automated user that's generating bad cookies.

I'll add some robustness tests for the utilities and patch them up to be on the safe side.