On Tue, Jan 27, 2015 at 5:07 AM, Hyrum K Wright <hyrum_at_hyrumwright.org>
> There's a stack overflow bug in subversion/libsubr/checksum.c.
Well, a segfault.
> The functions svn_checksum__from_digest_fnv1a_32x4() and
> svn_checksum__from_digest_fnv1a_32() both look something like this:
> svn_checksum_t *
> svn_checksum__from_digest_fnv1a_32x4(const unsigned char *digest,
> apr_pool_t *result_pool)
> return checksum_create(svn_checksum_fnv1a_32x4, sizeof(digest), digest,
> The problem is that checksum_create() expects the length of the string
> pointed to by digest, but sizeof(digest) returns the number of bytes that a
> variable of type 'const unsigned char *' requires. These methods are
> currently only called with unint32_t cast to an unsigned char, but on
> platforms which have 8-byte pointers, this leads to a buffer overflow in
> checksum_create() when it tries to read more than the provided 4 bytes.
It is a remnant of the original implementation that passed in a uint32
and a later change to char*.
> I *think* the correct fix is to add a digest_size argument
> to svn_checksum__from_digest_fnv1a_32x4() and
> svn_checksum__from_digest_fnv1a_32(), but I'm not sure if there's a better
> way which somebody more familiar with this code would know about.
The digest size is known just like in the functions for sha1 and md5.
Moreover, it is redundant with the checksum kind already given.
r1654981 fixes the problem.
> By all accounts, this code hasn't been released yet, so it's not a yet
> security issue (hence the post to dev@ and not private@), but I thought
> somebody would like to be aware of it. :)
Thanks for the review! Because the current callers would probably
never have triggered the segfault, this bug might have lingered in
there for years ...
Received on 2015-01-27 08:02:07 CET