8000 `parse_cookie_str` can be crashed by end users via `cookie_str = 'true'` · Issue #135 · jazzband/django-cookie-consent · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
gmseb opened this issue Feb 8, 2025 · 3 comments
Open

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

gmseb opened this issue Feb 8, 2025 · 3 comments

Comments

@gmseb
Copy link
gmseb commented Feb 8, 2025

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…

django-cookie-consent/cookie_consent/util.py

Line 15 in 81aae63

key, value = c.split("=")

…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
Copy link
Contributor

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
Copy link
869E

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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tomcheney @sergei-maertens @some1ataplace @gmseb
0