Opened 20 months ago

Closed 20 months ago

Last modified 20 months 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 20 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 20 months ago by Jan Pieter Waagmeester

Description: modified (diff)

comment:2 Changed 20 months ago by Mariusz Felisiak

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 Changed 20 months ago by Florian Apolloner

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 Changed 20 months ago by Florian Apolloner

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 Changed 20 months ago by Jan Pieter Waagmeester

I sent the contents of the cookie to security@

comment:6 Changed 20 months ago by Florian Apolloner

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

Changed 20 months ago by Florian Apolloner

Attachment: 32643.diff added

comment:7 Changed 20 months ago by 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?

comment:8 Changed 20 months ago by Mariusz Felisiak

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

comment:9 in reply to:  7 Changed 20 months ago by Jan Pieter Waagmeester

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 Changed 20 months ago by Mariusz Felisiak

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:11 Changed 20 months ago by Mariusz Felisiak <felisiak.mariusz@…>

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 Changed 20 months ago by Mariusz Felisiak <felisiak.mariusz@…>

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 Changed 20 months ago by Adam Hooper

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 20 months ago by Adam Hooper (previous) (diff)

comment:14 Changed 20 months ago by Florian Apolloner

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