gmp-utils: protect gdb_mpz exports against out-of-range values

The gdb_mpz class currently provides a couple of methods which
essentially export an mpz_t value into either a buffer, or an integral
type. The export is based on using the mpz_export function which
we discovered can be a bit treacherous if used without caution.

In particular, the initial motivation for this patch was to catch
situations where the mpz_t value was so large that it would not fit
in the destination area. mpz_export does not know the size of
the buffer, and therefore can happily write past the end of our buffer.

While designing a solution to the above problem, I also discovered
that we also needed to be careful when exporting signed numbers.
In particular, numbers which are larger than the maximum value
for a given signed type size, but no so large as to fit in the
*unsigned* version with the same size, would end up being exported
incorrectly. This is related to the fact that mpz_export ignores
the sign of the value being exportd, and assumes an unsigned export.
Thus, for such large values, the appears as if mpz_export is able
to fit our value into our buffer, but in fact, it does not.

Also, I noticed that gdb_mpz::write wasn't taking its unsigned_p
parameter, which was a hole.

For all these reasons, a new low-level private method called
"safe_export" has been added to class gdb_mpz, whose goal is
to perform all necessary checks and manipulations for a safe
and correct export. As a bonus, this method allows us to factorize
the handling of negative value exports.

The gdb_mpz::as_integer and gdb_mpz::write methods are then simplified
to take advantage of this new safe_export method.

gdb/ChangeLog:

        * gmp-utils.h (gdb_mpz::safe_export): New private method.
        (gdb_mpz::as_integer): Reimplement using gdb_mpz::safe_export.
        * gmp-utils.c (gdb_mpz::write): Rewrite using gdb_mpz::safe_export.
        (gdb_mpz::safe_export): New method.
        * unittests/gmp-utils-selftests .c (gdb_mpz_as_integer):
        Update function description.
        (check_as_integer_raises_out_of_range_error): New function.
        (gdb_mpz_as_integer_out_of_range): New function.
        (_initialize_gmp_utils_selftests): Register
        gdb_mpz_as_integer_out_of_range as a selftest.

1 files changed, 71 insertions, 8 deletions

diff --git a/gdb/gmp-utils.c b/gdb/gmp-utils.c
index e3a3333..60c915c 100644
--- a/gdb/gmp-utils.c
+++ b/gdb/gmp-utils.c
@@ -68,9 +68,61 @@ void
 gdb_mpz::write (gdb::array_view<gdb_byte> buf, enum bfd_endian byte_order,
 		bool unsigned_p) const
 {
+  this->safe_export
+    (buf, byte_order == BFD_ENDIAN_BIG ? 1 : -1 /* endian */, unsigned_p);
+}
+
+/* See gmp-utils.h.  */
+
+void
+gdb_mpz::safe_export (gdb::array_view<gdb_byte> buf,
+		      int endian, bool unsigned_p) const
+{
+  gdb_assert (buf.size () > 0);
+
+  if (mpz_sgn (val) == 0)
+    {
+      /* Our value is zero, so no need to call mpz_export to do the work,
+	 especially since mpz_export's documentation explicitly says
+	 that the function is a noop in this case.  Just write zero to
+	 BUF ourselves.  */
+      memset (buf.data (), 0, buf.size ());
+      return;
+    }
+
+  /* Determine the maximum range of values that our buffer can hold,
+     and verify that VAL is within that range.  */
+
+  gdb_mpz lo, hi;
+  const size_t max_usable_bits = buf.size () * HOST_CHAR_BIT;
+  if (unsigned_p)
+    {
+      lo = 0;
+
+      mpz_ui_pow_ui (hi.val, 2, max_usable_bits);
+      mpz_sub_ui (hi.val, hi.val, 1);
+    }
+  else
+    {
+      mpz_ui_pow_ui (lo.val, 2, max_usable_bits - 1);
+      mpz_neg (lo.val, lo.val);
+
+      mpz_ui_pow_ui (hi.val, 2, max_usable_bits - 1);
+      mpz_sub_ui (hi.val, hi.val, 1);
+    }
+
+  if (mpz_cmp (val, lo.val) < 0 || mpz_cmp (val, hi.val) > 0)
+    error (_("Cannot export value %s as %zu-bits %s integer"
+	     " (must be between %s and %s)"),
+	   this->str ().c_str (),
+	   max_usable_bits,
+	   unsigned_p ? _("unsigned") : _("signed"),
+	   lo.str ().c_str (),
+	   hi.str ().c_str ());
+
   gdb_mpz exported_val (val);
 
-  if (mpz_cmp_ui (val, 0) < 0)
+  if (mpz_cmp_ui (exported_val.val, 0) < 0)
     {
       /* mpz_export does not handle signed values, so create a positive
 	 value whose bit representation as an unsigned of the same length
@@ -81,13 +133,24 @@ gdb_mpz::write (gdb::array_view<gdb_byte> buf, enum bfd_endian byte_order,
       mpz_add (exported_val.val, exported_val.val, neg_offset.val);
     }
 
-  /* Start by clearing the buffer, as mpz_export only writes as many
-     bytes as it needs (including none, if the value to export is zero.  */
-  memset (buf.data (), 0, buf.size ());
-  mpz_export (buf.data (), NULL /* count */, -1 /* order */,
-	      buf.size () /* size */,
-	      byte_order == BFD_ENDIAN_BIG ? 1 : -1 /* endian */,
-	      0 /* nails */, exported_val.val);
+  /* Do the export into a buffer allocated by GMP itself; that way,
+     we can detect cases where BUF is not large enough to export
+     our value, and thus avoid a buffer overlow.  Normally, this should
+     never happen, since we verified earlier that the buffer is large
+     enough to accomodate our value, but doing this allows us to be
+     extra safe with the export.
+
+     After verification that the export behaved as expected, we will
+     copy the data over to BUF.  */
+
+  size_t word_countp;
+  gdb::unique_xmalloc_ptr<void> exported
+    (mpz_export (NULL, &word_countp, -1 /* order */, buf.size () /* size */,
+		 endian, 0 /* nails */, exported_val.val));
+
+  gdb_assert (word_countp == 1);
+
+  memcpy (buf.data (), exported.get (), buf.size ());
 }
 
 /* See gmp-utils.h.  */