darcs

Issue 1521 patch bundle hash check not done early enough to detect corruption

Title patch bundle hash check not done early enough to detect corruption
Priority bug Status given-up
Milestone Resolved in
Superseder Nosy List alain91, darcs-devel, dmitry.kurochkin, jaredj, kowey, mornfall, thorkilnaur, tux_rocker
Assigned To
Topics ProbablyEasy

Created on 2009-08-14.11:53:24 by kowey, last changed 2017-07-31.01:56:50 by gh.

Files
File name Uploaded Type Edit Remove
possibly-corrupt.dpatch.gz kowey, 2009-08-14.11:53:21 application/x-gzip
Messages
msg8136 (view) Author: kowey Date: 2009-08-14.11:53:21
Observation by Petr in issue1382, msg7413.

This requires a volunteer to study the code and figure out (1) where we think
we're performing the check and (2) whether it can be done earlier.

A good sanity check to begin with would be to take a patch bundle which we think
is corrupt and actually compare the hash to make sure.  If we do get a failure,
then we are more sure that this bug is legit
Attachments
msg8140 (view) Author: tux_rocker Date: 2009-08-14.19:58:27
For me, the hash check works.

A few days ago, I had a sudden urge to check if darcs still watches out for
malicious paths in patches. So I tried to craft a malicious bundle and I did
have to circumvent the hash check by changing the hash whenever I changed the
content.
msg8141 (view) Author: kowey Date: 2009-08-14.21:36:22
Hi Reinier, did you mean that you tried checking the possible-corrupt bundle I
had attached?  If so, how?  I imagine I still need to chop off bits of the
bundle before I try something like the sha1sum utility.

Or did you just mean your malicious bundle?
msg8142 (view) Author: mornfall Date: 2009-08-14.21:53:43
The problem is not that the hash check is not done. The problem is that a bad
bundle will crash darcs before it tries to check the hash, leaving the user
puzzled as to what's wrong (while it could just say "bad bundle"). Getting a
"darcs failed: bug in ..." whatever is not so great.
msg8165 (view) Author: tux_rocker Date: 2009-08-15.19:23:15
Eric Kow <kowey@darcs.net> added the comment:
> Hi Reinier, did you mean that you tried checking the possible-corrupt
> bundle I had attached?  If so, how?  I imagine I still need to chop off
> bits of the bundle before I try something like the sha1sum utility.
>
> Or did you just mean your malicious bundle?

I just meant my malicious bundle.
msg10493 (view) Author: kowey Date: 2010-03-24.17:56:32
Seems like we just need a volunteer/round-tuit to modify darcs apply so
that it complains about patch bundle hash mismatch first before trying
to do anything else.

Golly, that doesn't sound too hard.
msg14644 (view) Author: kowey Date: 2011-08-13.16:28:13
Huh? We could not reproduce the hash check insufficiency.  We noted that 
if we do wibble the patch bundle hash to something else, we get a bad hash 
warning.  Are we sure this is really the problem here, at least for 
issue1382?
History
Date User Action Args
2009-08-14 11:53:24koweycreate
2009-08-14 19:58:30tux_rockersetnosy: + tux_rocker
messages: + msg8140
2009-08-14 21:36:24koweysetstatus: needs-reproduction -> waiting-for
nosy: kowey, simon, thorkilnaur, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8141
2009-08-14 21:53:45mornfallsetnosy: kowey, simon, thorkilnaur, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8142
2009-08-14 21:56:14mornfalllinkissue1382 superseder
2009-08-15 19:23:17tux_rockersetnosy: kowey, simon, thorkilnaur, tux_rocker, dmitry.kurochkin, mornfall
messages: + msg8165
2009-08-25 18:15:03adminsetnosy: + darcs-devel, - simon
2009-08-27 14:24:42adminsetnosy: kowey, darcs-devel, thorkilnaur, tux_rocker, dmitry.kurochkin, mornfall
2010-03-24 17:56:36koweysetstatus: waiting-for -> needs-implementation
nosy: + jaredj
topic: + ProbablyEasy
messages: + msg10493
2011-08-13 16:28:14koweysetstatus: needs-implementation -> waiting-for
messages: + msg14644
2015-05-09 20:40:46alain91setassignedto: alain91
nosy: + alain91
2015-05-23 18:46:58alain91setassignedto: alain91 ->
2017-07-31 01:56:50ghsetstatus: waiting-for -> given-up