On Wed, Dec 11, 2013 at 3:13 PM, Nikos Mavrogiannopoulos
<[email protected]> wrote:

> As far as I understand that size assumption is only enforced on
> systems without alloca using an assert. In systems with alloca there
> is no such check. In both cases, it seems to be easy to abuse them for
> a denial of service.
> I think it would be better for these functions to fail rather than
> abort() if parameters are out of supported range.

A fix that could suit the master branch is attached. That adds the
possibility to return an error if the maximum sizes are exceeded. For
2.7.x though this can be handled by an abort which is less than ideal,
as it looks trivial to exploit for DoS. Even worse,
nettle_mpz_set_str_256*() does not enforce the maximum limits on big
numbers and doesn't even have the ability to return an error.

The patch is pretty ugly because it introduces unused variables, but
couldn't think of a cleaner way to fix that.

Any other ideas on how the issue can be gracefully solved (especially
in the 2.7 branch)?

regards,
Nikos
diff --git a/bignum-next-prime.c b/bignum-next-prime.c
index 58a4df8..9c17e09 100644
--- a/bignum-next-prime.c
+++ b/bignum-next-prime.c
@@ -72,7 +72,7 @@ static const uint16_t primes[] = {
 #endif
 
 /* NOTE: The mpz_nextprime in current GMP is unoptimized. */
-void
+int
 nettle_next_prime(mpz_t p, mpz_t n, unsigned count, unsigned prime_limit,
                  void *progress_ctx, nettle_progress_func *progress)
 {
@@ -88,13 +88,13 @@ nettle_next_prime(mpz_t p, mpz_t n, unsigned count, 
unsigned prime_limit,
   if (mpz_cmp_ui(n, 2) <= 0)
     {
       mpz_set_ui(p, 2);
-      return;
+      return 1;
     }
   mpz_set(p, n);
   mpz_setbit(p, 0);
 
   if (mpz_cmp_ui(p, 8) < 0)
-    return;
+    return 1;
 
   mpz_init(tmp);
 
@@ -112,7 +112,7 @@ nettle_next_prime(mpz_t p, mpz_t n, unsigned count, 
unsigned prime_limit,
      between the 5760 odd numbers in this interval that have no factor
      in common with 15015.
    */
-  TMP_ALLOC(moduli, prime_limit);
+  TMP_ALLOC_SAFE_ERR(moduli, prime_limit);
   {
     unsigned i;
     for (i = 0; i < prime_limit; i++)
@@ -159,4 +159,6 @@ nettle_next_prime(mpz_t p, mpz_t n, unsigned count, 
unsigned prime_limit,
 #endif
     }
   mpz_clear(tmp);
+  
+  return 1;
 }
diff --git a/bignum-random.c b/bignum-random.c
index f305f04..a584ba2 100644
--- a/bignum-random.c
+++ b/bignum-random.c
@@ -32,14 +32,14 @@
 #include "bignum.h"
 #include "nettle-internal.h"
 
-void
+int
 nettle_mpz_random_size(mpz_t x,
                       void *ctx, nettle_random_func *random,
                       unsigned bits)
 {
   unsigned length = (bits + 7) / 8;
   TMP_DECL(data, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(data, length);
+  TMP_ALLOC_SAFE_ERR(data, length);
 
   random(ctx, length, data);
 
@@ -47,14 +47,17 @@ nettle_mpz_random_size(mpz_t x,
 
   if (bits % 8)
     mpz_fdiv_r_2exp(x, x, bits);
+  
+  return 1;
 }
 
 /* Returns a random number x, 0 <= x < n */
-void
+int
 nettle_mpz_random(mpz_t x,
                  void *ctx, nettle_random_func *random,
                  const mpz_t n)
 {
+int ret;
   /* NOTE: This leaves some bias, which may be bad for DSA. A better
    * way might be to generate a random number of mpz_sizeinbase(n, 2)
    * bits, and loop until one smaller than n is found. */
@@ -78,9 +81,12 @@ nettle_mpz_random(mpz_t x,
    * operation. NIST FIPS 186-3 specifies 64 extra bits, for use with
    * DSA. */
 
-  nettle_mpz_random_size(x, 
+  ret = nettle_mpz_random_size(x, 
                         ctx, random,
                         mpz_sizeinbase(n, 2) + 64);
-  
+  if (ret == 0)
+    return ret;
   mpz_fdiv_r(x, x, n);
+  
+  return 1;
 }
diff --git a/bignum.h b/bignum.h
index b5b9f1a..039b64c 100644
--- a/bignum.h
+++ b/bignum.h
@@ -69,19 +69,19 @@ nettle_mpz_init_set_str_256_u(mpz_t x,
                              size_t length, const uint8_t *s);
 
 /* Returns a uniformly distributed random number 0 <= x < 2^n */
-void
+int
 nettle_mpz_random_size(mpz_t x,
                       void *ctx, nettle_random_func *random,
                       unsigned bits);
 
 /* Returns a number x, almost uniformly random in the range
  * 0 <= x < n. */
-void
+int
 nettle_mpz_random(mpz_t x, 
                  void *ctx, nettle_random_func *random,
                  const mpz_t n);
 
-void
+int
 nettle_next_prime(mpz_t p, mpz_t n, unsigned count, unsigned prime_limit,
                  void *progress_ctx, nettle_progress_func *progress);
 
diff --git a/nettle-internal.h b/nettle-internal.h
index 10689d4..4af90f3 100644
--- a/nettle-internal.h
+++ b/nettle-internal.h
@@ -37,12 +37,25 @@
  * fix maximum size, and abort if we ever need anything larger. */
 
 #if HAVE_ALLOCA
-# define TMP_DECL(name, type, max) type *name
-# define TMP_ALLOC(name, size) (name = alloca(sizeof (*name) * (size)))
+# define TMP_DECL(name, type, max) \
+       type *name; \
+       static const unsigned name##_max = max
+# define TMP_ALLOC_SAFE_ERR(name, size) \
+       do { \
+       if (name##_max < (sizeof (*name) * (size))) \
+               return 0; \
+       (name = alloca(sizeof (*name) * (size))); \
+       } while (0)
+# define TMP_ALLOC(name, size) \
+       (name = alloca(sizeof (*name) * (size)))
 #else /* !HAVE_ALLOCA */
-# define TMP_DECL(name, type, max) type name[max]
+# define TMP_DECL(name, type, max) \
+       type name[max]; \
+       const unsigned name##_tmp_max = max
 # define TMP_ALLOC(name, size) \
   do { if ((size) > (sizeof(name) / sizeof(name[0]))) abort(); } while (0)
+# define TMP_ALLOC_SAFE_ERR(name, size) \
+  do { if ((size) > (sizeof(name) / sizeof(name[0]))) return 0; } while (0)
 #endif 
 
 /* Arbitrary limits which apply to systems that don't have alloca */
diff --git a/pkcs1-decrypt.c b/pkcs1-decrypt.c
index 02d3728..b685b09 100644
--- a/pkcs1-decrypt.c
+++ b/pkcs1-decrypt.c
@@ -43,7 +43,7 @@ pkcs1_decrypt (size_t key_size,
   size_t padding;
   size_t message_length;
 
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
   nettle_mpz_get_str_256(key_size, em, m);
 
   /* Check format */
diff --git a/pkcs1-encrypt.c b/pkcs1-encrypt.c
index 69ef5bc..cceeeca 100644
--- a/pkcs1-encrypt.c
+++ b/pkcs1-encrypt.c
@@ -63,7 +63,7 @@ pkcs1_encrypt (size_t key_size,
   padding = key_size - length - 3;
   assert(padding >= 8);
   
-  TMP_ALLOC(em, key_size - 1);
+  TMP_ALLOC_SAFE_ERR(em, key_size - 1);
   em[0] = 2;
 
   random(random_ctx, padding, em + 1);
diff --git a/pkcs1-rsa-digest.c b/pkcs1-rsa-digest.c
index debfb28..35b4777 100644
--- a/pkcs1-rsa-digest.c
+++ b/pkcs1-rsa-digest.c
@@ -36,7 +36,7 @@ pkcs1_rsa_digest_encode(mpz_t m, size_t key_size,
                        size_t di_length, const uint8_t *digest_info)
 {
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   if (_pkcs1_signature_prefix(key_size, em,
                              di_length, digest_info, 0))
diff --git a/pkcs1-rsa-md5.c b/pkcs1-rsa-md5.c
index b118b4f..440242a 100644
--- a/pkcs1-rsa-md5.c
+++ b/pkcs1-rsa-md5.c
@@ -66,7 +66,7 @@ pkcs1_rsa_md5_encode(mpz_t m, size_t key_size, struct md5_ctx 
*hash)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(md5_prefix),
@@ -87,7 +87,7 @@ pkcs1_rsa_md5_encode_digest(mpz_t m, size_t key_size, const 
uint8_t *digest)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(md5_prefix),
diff --git a/pkcs1-rsa-sha1.c b/pkcs1-rsa-sha1.c
index 781d75d..3996ecf 100644
--- a/pkcs1-rsa-sha1.c
+++ b/pkcs1-rsa-sha1.c
@@ -66,7 +66,7 @@ pkcs1_rsa_sha1_encode(mpz_t m, size_t key_size, struct 
sha1_ctx *hash)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha1_prefix),
@@ -87,7 +87,7 @@ pkcs1_rsa_sha1_encode_digest(mpz_t m, size_t key_size, const 
uint8_t *digest)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha1_prefix),
diff --git a/pkcs1-rsa-sha256.c b/pkcs1-rsa-sha256.c
index a4d5bb1..12d355d 100644
--- a/pkcs1-rsa-sha256.c
+++ b/pkcs1-rsa-sha256.c
@@ -64,7 +64,7 @@ pkcs1_rsa_sha256_encode(mpz_t m, size_t key_size, struct 
sha256_ctx *hash)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha256_prefix),
@@ -85,7 +85,7 @@ pkcs1_rsa_sha256_encode_digest(mpz_t m, size_t key_size, 
const uint8_t *digest)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha256_prefix),
diff --git a/pkcs1-rsa-sha512.c b/pkcs1-rsa-sha512.c
index 03acb69..e6f54f1 100644
--- a/pkcs1-rsa-sha512.c
+++ b/pkcs1-rsa-sha512.c
@@ -64,7 +64,7 @@ pkcs1_rsa_sha512_encode(mpz_t m, size_t key_size, struct 
sha512_ctx *hash)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha512_prefix),
@@ -85,7 +85,7 @@ pkcs1_rsa_sha512_encode_digest(mpz_t m, size_t key_size, 
const uint8_t *digest)
 {
   uint8_t *p;
   TMP_DECL(em, uint8_t, NETTLE_MAX_BIGNUM_SIZE);
-  TMP_ALLOC(em, key_size);
+  TMP_ALLOC_SAFE_ERR(em, key_size);
 
   p = _pkcs1_signature_prefix(key_size, em,
                              sizeof(sha512_prefix),
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs

Reply via email to