Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32643 closed Bug (fixed)

CookieStorage for contrib.messages crashes after upgrade to django 3.2

Reported by: Jan Pieter Waagmeester Owned by: Florian Apolloner
Component: contrib.messages Version: 3.2
Severity: Release blocker Keywords:
Cc: Florian Apolloner Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Jan Pieter Waagmeester)

After upgrading to django 3.2, a previously stored cookie for contrib.messages crashes in

https://github.com/django/django/blob/d6314c4c2ef647efe0d12450214fc5b4a4055290/django/contrib/messages/storage/cookie.py#L175

Django Version: 3.2
Python Version: 3.8.2

Traceback (most recent call last):
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/utils/deprecation.py", line 119, in __call__
    response = self.process_response(request, response)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/middleware.py", line 23, in process_response
    unstored_messages = request._messages.update(response)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/storage/base.py", line 127, in update
    messages = self._loaded_messages + self._queued_messages
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/storage/base.py", line 79, in _loaded_messages
    messages, all_retrieved = self._get()
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/storage/fallback.py", line 25, in _get
    messages, all_retrieved = storage._get()
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/storage/cookie.py", line 86, in _get
    messages = self._decode(data)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/contrib/messages/storage/cookie.py", line 175, in _decode
    return self.signer.unsign_object(data, serializer=MessageSerializer)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/core/signing.py", line 195, in unsign_object
    data = b64_decode(base64d)
  File "/home/obs/virtualenv/lib/python3.8/site-packages/django/core/signing.py", line 68, in b64_decode
    return base64.urlsafe_b64decode(s + pad)
  File "/usr/lib/python3.8/base64.py", line 133, in urlsafe_b64decode
    return b64decode(s)
  File "/usr/lib/python3.8/base64.py", line 87, in b64decode
    return binascii.a2b_base64(s)

Exception Type: Error at /user/login/
Exception Value: Invalid base64-encoded string: number of data characters (369) cannot be 1 more than a multiple of 4

(redacted) contents of the 'messages' cookie:

'[["__json_message",0,25,"Successfully signed in as '
 'admin@example.org."],["__json_message",0,25,"Successfully '
 'signed in as jieter."],["__json_message",0,25,"Ingelogd als '
 'admin@example.org."],["__json_message",0,25,"Ingelogd '
 'als '
 'admin@example.org."],["__json_message",0,20,"Bevestigingsmail '
 'verzonden naar test@example.nl."],["__json_message",0,25,"Ingelogd '
 'als '
 'test@example.nl."]]:1lTkj1:j_3PlpYSKiqPTMAB6_p2Q00eE8j6k7n0Sg_-_IpXG7Y')

Attachments (1)

32643.diff (2.5 KB ) - added by Florian Apolloner 3 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 by Jan Pieter Waagmeester, 3 years ago

Description: modified (diff)

comment:2 by Mariusz Felisiak, 3 years ago

Cc: Florian Apolloner added

Jan thanks for this report, however I cannot reproduce this issue. Everything works when I will use your messages in messages_tests.test_cookie.CookieTests.test_legacy_encode_decode.

comment:3 by Florian Apolloner, 3 years ago

We might need the original cookie as seen by the browser. Can you send it to security@… (for the lack of a better "trusted" address, we will handle the data with care).

comment:4 by Florian Apolloner, 3 years ago

What confuses me here is that according to the traceback the signature was valid and it then failed in https://github.com/django/django/blob/b6475d7d7940f3ce575e0b0f2d83e517f899b4cf/django/core/signing.py#L195 -- but if the signature was valid in the first place the data should have been base64 encoded. The contents of the cookie seem to suggest otherwise.

comment:5 by Jan Pieter Waagmeester, 3 years ago

I sent the contents of the cookie to security@

comment:6 by Florian Apolloner, 3 years ago

Thanks, I think I understand what is going on. Out of pure luck we created test messages that properly decode from base64. They then fail json decoding and fall back to the old mechanism. I think we need to add binascii.Error to https://github.com/django/django/commit/2d6179c819010f6a9d00835d5893c4593c0b85a0#diff-6c5e944e6869d04c50af2ee02dd85c621177547eb7771d11066f452788d483f4R185

by Florian Apolloner, 3 years ago

Attachment: 32643.diff added

comment:7 by Florian Apolloner, 3 years ago

Hi Jan, I have attached a diff that should fix your issue. Do you think you could apply that single change to your Django codebase and retry?

comment:8 by Mariusz Felisiak, 3 years ago

Owner: changed from nobody to Florian Apolloner
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted
Last edited 3 years ago by Mariusz Felisiak (previous) (diff)

in reply to:  7 comment:9 by Jan Pieter Waagmeester, 3 years ago

Replying to Florian Apolloner:

Hi Jan, I have attached a diff that should fix your issue. Do you think you could apply that single change to your Django codebase and retry?

Works like expected with the patch applied, great!

comment:10 by Mariusz Felisiak, 3 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 4511d145:

Fixed #32643 -- Fixed decoding of messages in the pre-Django 3.2 format.

Thanks Jan Pieter Waagmeester for the report.

Regression in 2d6179c819010f6a9d00835d5893c4593c0b85a0.

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 539d005a:

[3.2.x] Fixed #32643 -- Fixed decoding of messages in the pre-Django 3.2 format.

Thanks Jan Pieter Waagmeester for the report.

Regression in 2d6179c819010f6a9d00835d5893c4593c0b85a0.

Backport of 4511d1459810037b91faa5b506e4f75c77aa72be from main.

comment:13 by Adam Hooper, 3 years ago

Could Django please release a patch release that fixes this bug?

Unless I'm misunderstanding, I've run into this:

  1. Upgrade to Django 3.2.
  2. Test. Everything passes.
  3. Deploy to production.
  4. Users experience 500 errors.

In my case, I happened to catch this when I deployed to staging. Pure luck.

But in general, this crash affects tons of existing sites and there is no workaround until 3.2.1 is released, right?

Last edited 3 years ago by Adam Hooper (previous) (diff)

comment:14 by Florian Apolloner, 3 years ago

and there is no workaround until 3.2.1 is released, right?

Setting a custom storage backend with the fix applied is one option. The other option is a custom Django package till 3.2.1 is released.

Note: See TracTickets for help on using tickets.
Back to Top