From 862cf19a119427dd7ee7959a36c72d905f5ea5ca Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 1 Apr 2016 13:42:01 +0200 Subject: mpi: Explicitly limit the allowed input length for gcry_mpi_scan. * mpi/mpicoder.c (MAX_EXTERN_SCAN_BYTES): New. (mpi_fromstr): Check against this limit. (_gcry_mpi_scan): Ditto. * tests/mpitests.c (test_maxsize): New. (main): Cal that test. -- A too large buffer length may lead to an unsigned integer overflow on systems where size_t > unsigned int (ie. 64 bit systems). The computation of the required number of nlimbs may also be affected by this. However this is not a real world case because any processing which has allocated such a long buffer from an external source would be prone to other DoS attacks: The required buffer length to exhibit this overflow is at least 2^32 - 8 bytes. Signed-off-by: Werner Koch --- doc/gcrypt.texi | 4 +++- mpi/mpicoder.c | 34 ++++++++++++++++++++++++++++++---- tests/mpitests.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 5 deletions(-) diff --git a/doc/gcrypt.texi b/doc/gcrypt.texi index 437dddb7..c710765b 100644 --- a/doc/gcrypt.texi +++ b/doc/gcrypt.texi @@ -4526,7 +4526,8 @@ representation of an MPI and the internal one of Libgcrypt. Convert the external representation of an integer stored in @var{buffer} with a length of @var{buflen} into a newly created MPI returned which will be stored at the address of @var{r_mpi}. For certain formats the -length argument is not required and should be passed as @code{0}. After a +length argument is not required and should be passed as @code{0}. A +@var{buflen} larger than 16 MiByte will be rejected. After a successful operation the variable @var{nscanned} receives the number of bytes actually scanned unless @var{nscanned} was given as @code{NULL}. @var{format} describes the format of the MPI as stored in @@ -4540,6 +4541,7 @@ bytes actually scanned unless @var{nscanned} was given as @item GCRYMPI_FMT_PGP As used by OpenPGP (only defined as unsigned). This is basically @code{GCRYMPI_FMT_STD} with a 2 byte big endian length header. +A length header indicating a length of more than 16384 is not allowed. @item GCRYMPI_FMT_SSH As used in the Secure Shell protocol. This is @code{GCRYMPI_FMT_STD} diff --git a/mpi/mpicoder.c b/mpi/mpicoder.c index 896dda14..e3155766 100644 --- a/mpi/mpicoder.c +++ b/mpi/mpicoder.c @@ -27,8 +27,21 @@ #include "mpi-internal.h" #include "g10lib.h" +/* The maximum length we support in the functions converting an + * external representation to an MPI. This limit is used to catch + * programming errors and to avoid DoS due to insane long allocations. + * The 16 MiB limit is actually ridiculous large but some of those PQC + * algorithms use quite large keys and they might end up using MPIs + * for that. */ +#define MAX_EXTERN_SCAN_BYTES (16*1024*1024) + +/* The maximum length (in bits) we support for OpenPGP MPIs. Note + * that OpenPGP's MPI format uses only two bytes and thus would be + * limited to 64k anyway. Note that this limit matches that used by + * GnuPG. */ #define MAX_EXTERN_MPI_BITS 16384 + /* Helper used to scan PGP style MPIs. Returns NULL on failure. */ static gcry_mpi_t mpi_read_from_buffer (const unsigned char *buffer, unsigned *ret_nread, @@ -104,7 +117,13 @@ mpi_fromstr (gcry_mpi_t val, const char *str) if ( *str == '0' && str[1] == 'x' ) str += 2; - nbits = 4 * strlen (str); + nbits = strlen (str); + if (nbits > MAX_EXTERN_SCAN_BYTES) + { + mpi_clear (val); + return 1; /* Error. */ + } + nbits *= 4; if ((nbits % 8)) prepend_zero = 1; @@ -438,9 +457,9 @@ twocompl (unsigned char *p, unsigned int n) /* Convert the external representation of an integer stored in BUFFER - with a length of BUFLEN into a newly create MPI returned in - RET_MPI. If NBYTES is not NULL, it will receive the number of - bytes actually scanned after a successful operation. */ + * with a length of BUFLEN into a newly create MPI returned in + * RET_MPI. If NSCANNED is not NULL, it will receive the number of + * bytes actually scanned after a successful operation. */ gcry_err_code_t _gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, const void *buffer_arg, size_t buflen, size_t *nscanned) @@ -450,6 +469,13 @@ _gcry_mpi_scan (struct gcry_mpi **ret_mpi, enum gcry_mpi_format format, unsigned int len; int secure = (buffer && _gcry_is_secure (buffer)); + if (buflen > MAX_EXTERN_SCAN_BYTES) + { + if (nscanned) + *nscanned = 0; + return GPG_ERR_INV_OBJ; + } + if (format == GCRYMPI_FMT_SSH) len = 0; else diff --git a/tests/mpitests.c b/tests/mpitests.c index e6f8525c..b6630291 100644 --- a/tests/mpitests.c +++ b/tests/mpitests.c @@ -236,6 +236,42 @@ test_opaque (void) } +static void +test_maxsize (void) +{ + gpg_error_t err; + gcry_mpi_t a; + char buffer[2+2048]; /* For PGP: 2 length bytes and 16384 bits. */ + + memset (buffer, 0x55, sizeof buffer); + + /* We use a short buffer but a give a too large length to simulate a + * programming error. In case this test fails (i.e. die() is + * called) the scan function may have access data outside of BUFFER + * which may result in a segv but we ignore that to avoid actually + * allocating such a long buffer. */ + err = gcry_mpi_scan (&a, GCRYMPI_FMT_USG, buffer, 16*1024*1024 +1, NULL); + if (gpg_err_code (err) != GPG_ERR_INV_OBJ) + die ("gcry_mpi_scan does not detect its generic input limit\n"); + + /* Now test the PGP limit. The scan code check the two length bytes + * from the buffer and thus it is sufficient to fake them. */ + buffer[0] = (16385 >> 8); + buffer[1] = (16385 & 0xff); + err = gcry_mpi_scan (&a, GCRYMPI_FMT_PGP, buffer, sizeof buffer, NULL); + if (gpg_err_code (err) != GPG_ERR_INV_OBJ) + die ("gcry_mpi_scan does not detect the PGP input limit\n"); + + buffer[0] = (16384 >> 8); + buffer[1] = (16384 & 0xff); + + err = gcry_mpi_scan (&a, GCRYMPI_FMT_PGP, buffer, sizeof buffer, NULL); + if (err) + die ("gcry_mpi_scan did not parse a large PGP: %s\n", gpg_strerror (err)); + gcry_mpi_release (a); +} + + static void test_cmp (void) { @@ -569,6 +605,7 @@ main (int argc, char* argv[]) test_const_and_immutable (); test_opaque (); + test_maxsize (); test_cmp (); test_add (); test_sub (); -- cgit v1.2.1