Subject: bug#13827: faulty range check in bytevector accessor

From: "Ben Rocer" <[email-masked]>
Date: Mon, 28 Jul 2014 16:35:15 +0200

[resubmitting to [email-masked] as debbugs seems to have eaten my
 first mail]

When I tried to reproduce this bug on a 32-bit x86 system, I got an
abort in the function bytevector_large_set(); I think this is also
where the bug is.

Specifically, there are two bugs in these two consecutive lines in
bytevector_large_set():

value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
if (SCM_UNLIKELY (value_size > c_size))

In the first line, there is an off-by-one error in the calculation of
value_size; it gives the wrong answer if mpz_sizeinbase() is a
multiple of (8 * c_size) (see
https://gmplib.org/manual/Integer-Import-and-Export.html).

Secondly, this calculation gives the number of (c_size-byte) *words*
required to hold c_mpz, not the number of bytes. So the check in the
next line should be (c_size * value_size > c_size), or equivalently
(value_size > 1).

Since bytevector-u64-set! also calls bytevector_large_set, it
may be possible to reproduce this bug on 64 bit systems too; e.g
(bytevector-u64-set! (make-bytevector 8) 0 (expt 2 64) (endianness big))
[untested]

--- a/libguile/bytevectors.c
+++ b/libguile/bytevectors.c
@@ -867,10 +867,10 @@ bytevector_large_set (char *c_bv, size_t c_size, int signed_p,
     memset (c_bv, 0, c_size);
   else
     {
-      size_t word_count, value_size;
+      size_t word_count, value_words;
 
-      value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size);
-      if (SCM_UNLIKELY (value_size > c_size))
+      value_words = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size) - 1) / (8 * c_size);
+      if (SCM_UNLIKELY (value_words > 1))
 	{
 	  err = -2;
 	  goto finish;