#12273 closed (worksforme)
django.contrib.formtools.utils.security_hash resulting in different pickled string on same data
Reported by: | Rob Hudson | Owned by: | kenseehart |
---|---|---|---|
Component: | contrib.formtools | Version: | 1.1 |
Severity: | Keywords: | security hash | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a fairly complex form wizard I'm using to walk a site administrator through creating a bi-weekly newsletter.
On the "Done" step of the form wizard I was getting hash errors and started to dig a little deeper. In the security_hash method I output both the data and the pickled string and found that the data was the same but the pickled string differed by a small number of characters -- what looks to me like some quoting delimiter characters.
From Step 2 to Step 3, my print statements in security_hash shows me this (wrapped to not make reading this difficult):
[('fishing_message', u'<p>F</p>'), ('hunting_message', u'<p>H</p>'), ('special_fishing_header_1', u'F1'), ('special_fishing_message_1', u'<p>1</p>'), ('special_fishing_books_1', u'1-57223-365-6'), ('special_fishing_header_2', u'F2'), ('special_fishing_message_2', u'<p>2</p>'), ('special_fishing_books_2', u'1-57223-365-6'), ('special_hunting_header_1', u'H1'), ('special_hunting_message_1', u'<p>1</p>'), ('special_hunting_books_1', u'1-57223-365-6'), ('special_hunting_header_2', u'H2'), ('special_hunting_message_2', u'<p>2</p>'), ('special_hunting_books_2', u'1-57223-365-6'), '3&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8'] ?]q(Ufishing_messageq<p>F</p>q?qUhunting_messageq<p>H</p>q?qUspecial_fishing_header_1XF1q ?q Uspecial_fishing_message_1q <p>1</p>q 1-57223-365-6q*?q+U23&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8q,e.age_2q&<p>2</p>q'?q(Uspecial_hunting_books_2q)X
From Step 3 to Done, this is the output of the same data and pickled string:
[('fishing_message', u'<p>F</p>'), ('hunting_message', u'<p>H</p>'), ('special_fishing_header_1', u'F1'), ('special_fishing_message_1', u'<p>1</p>'), ('special_fishing_books_1', u'1-57223-365-6'), ('special_fishing_header_2', u'F2'), ('special_fishing_message_2', u'<p>2</p>'), ('special_fishing_books_2', u'1-57223-365-6'), ('special_hunting_header_1', u'H1'), ('special_hunting_message_1', u'<p>1</p>'), ('special_hunting_books_1', u'1-57223-365-6'), ('special_hunting_header_2', u'H2'), ('special_hunting_message_2', u'<p>2</p>'), ('special_hunting_books_2', u'1-57223-365-6'), '3&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8'] ?]q(Ufishing_messageq<p>F</p>q?qUhunting_messageq<p>H</p>q?qUspecial_fishing_header_1XF1q ?q Uspecial_fishing_message_1q <p>1</p>q 1-57223-365-6q'?q(U23&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8q)e._2q#<p>2</p>q$?q%Uspecial_hunting_books_2q&X
Trying to highlight the differences, they appear to be only on the last line of the pickled strings above, so here they are together with another line pointing at the differences:
1-57223-365-6q*?q+U23&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8q,e.age_2q&<p>2</p>q'?q(Uspecial_hunting_books_2q)X 1-57223-365-6q'?q(U23&#xsvm^bt(*tg-18-(b+3bfr+*21=c(gg8mnoyumgtd3!rok8q)e. _2q#<p>2</p>q$?q%Uspecial_hunting_books_2q&X ^ ^ ^ ___ ^ ^ ^ ^
In order to move past this I overrode the security_hash
method of my FormWizard to simply use repr(data)
rather than call the formtools version. I don't know if this is any more or less secure but since this is all happening in the admin I'm comfortable with less security rather than false positives.
Since it seems that this data is never un-pickled and the pickling is only there to generate a string that can be hashed, would it make sense to use repr()
instead of pickle? There are a few other bugs in the tracker related to the security_hash and pickled data this might solve.
Change History (5)
comment:1 by , 15 years ago
milestone: | → 1.2 |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 15 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 15 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Resolution: | → worksforme |
Status: | assigned → closed |
There is not enough information to reproduce the error. Ideally, we need enough to create a test case that fails.
Comments like "... From Step 2 to Step 3" are meaningless since there is no description of a sequence of steps.
In the case described, it appears that only basic python types are used (lists, tuples, strings), but if that were the case, then I can't see how it would be possible for pickle to give two different results when repr gives the same result. So it seems that some of the objects have repr that looks like a string repr.
I am not sure that replacing pickle with repr is wise. Using repr on such objects is dangerous because some objects could contain content that would cause problems for repr, whereas they would not cause problems for pickle. I don't see this as a security issue; just a possible cause of exceptions.
Some kinds of objects, such as dictionaries, can differ in both repr and pickle for equal values, so repr is not necessarily an improvement over pickle in the general case.
Therefore a description of how to cause this bug starting from a new Django installation is necessary.
This bug can be reopened if a sequence of steps can be provided that allows us to reproduce the problem on a fresh Django install.
Note: the current test suite does not cover utils.security_hash at all.
Implementing suggested change: "use repr() instead of pickle". If anyone knows a reason not to do this, please say so ASAP.