md5:md5sum-sequence interface change #27

Closed
esessoms opened this Issue Dec 2, 2012 · 5 comments

Projects

None yet

2 participants

@esessoms
esessoms commented Dec 2, 2012

The latest "release" of md5 (dated 20121125 according to quicklisp) will silently cast strings to byte arrays, yielding incorrect checksums and causing cl-postgres to fail to authenticate.

; incorrect

(md5:md5sum-sequence "usernamepassword")

(156 70 51 25 240 79 156 125 48 65 54 141 64 195 204 207)

; correct

(md5:md5sum-sequence (ironclad:ascii-string-to-byte-array "usernamepassword"))

(213 28 154 126 147 83 116 106 96 32 249 96 45 69 41 41)

(ironclad:digest-sequence :md5 (ironclad:ascii-string-to-byte-array "usernamepassword"))

(213 28 154 126 147 83 116 106 96 32 249 96 45 69 41 41)

Tested in SBCL 1.1.1 on MacOSX and Linux.

@esessoms esessoms referenced this issue in quicklisp/quicklisp-projects Dec 2, 2012
Closed

md5 update in 2012-11-25 breaks postmodern in sbcl #422

@marijnh
Owner
marijnh commented Dec 3, 2012

I can't reproduce this. Getting the latest md5 code from git, which is probably the same that you have, since the last commit was on 20121123, if I do (md5:md5sum-sequence "usernamepassword") I get the correct hash (ending in 41 41). I tried with both SBCL 1.0.54 and 1.1.2.

@esessoms
esessoms commented Dec 3, 2012

Perhaps we're using different md5 implementations? I'm running the one by Pierre Mai which is what quicklisp pulls in by default. Anyway, and sorry I didn't notice this earlier, there is a comment in the source that explains the situation:

https://github.com/pmai/md5/blob/master/md5.lisp#L620

Had I read that, I would have just sent you the patch rather that bothering you with a bug report. md5sum-STRING does get the job done.

@marijnh
Owner
marijnh commented Dec 3, 2012

I'm using that same implementation. Not sure why we're getting different results.

I don't want to pull in the (huge) dependency of Ironclad. Do you agree that simply calling md5sum-string is a proper fix?

@esessoms
esessoms commented Dec 3, 2012

I'm running 64-bit SBCL with unicode enabled. I always build with "fancy" these days and my value of char-code-limit is 1114112. Maybe that is the difference?

md5sum-string is at least as correct as calling ascii-string-to-byte-array, and probably more so. :) I've tested it on both my platforms, and reviewed the source for both it and the underlying sb-ext:string-to-octets. Yes, I understand not wanting to pull in an extra dependency which is why I went with issue and not patch... I just overlooked the ready-made solution right in front of me.

Thanks much!

@marijnh marijnh closed this in cd8c558 Dec 3, 2012
@marijnh
Owner
marijnh commented Dec 3, 2012

I'm on 64-bit as well, but my char-code-limit is 65536. Still, how that could influence the conversion of an ASCII string to octets is unclear to me.

I've pushed a slightly different patch. There is a perfectly serviceable string-to-byte-array utility in the Postmodern source already, so I simply called that.

@galdor galdor added a commit to galdor/Postmodern that referenced this issue May 6, 2013
@marijnh @galdor + galdor Don't pass a string to md5:md5sum-sequence
Closes #27
714a9c1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment