From cb37aa5912d9d172dd4f252bed37b7f7447816a8 Mon Sep 17 00:00:00 2001
From: Paul Bakker
Date: Wed, 30 Nov 2011 16:00:20 +0000
Subject: [PATCH] - Better buffer handling in mpi_read_file()
---
include/polarssl/bignum.h | 22 +++++++++++++++++-----
library/bignum.c | 8 ++++++--
tests/data_files/mpi_too_big | 1 +
tests/suites/test_suite_mpi.data | 4 ++++
4 files changed, 28 insertions(+), 7 deletions(-)
create mode 100644 tests/data_files/mpi_too_big
diff --git a/include/polarssl/bignum.h b/include/polarssl/bignum.h
index 12435f728..b4496b847 100644
--- a/include/polarssl/bignum.h
+++ b/include/polarssl/bignum.h
@@ -33,7 +33,7 @@
#define POLARSSL_ERR_MPI_FILE_IO_ERROR -0x0002 /**< An error occurred while reading from or writing to a file. */
#define POLARSSL_ERR_MPI_BAD_INPUT_DATA -0x0004 /**< Bad input parameters to function. */
#define POLARSSL_ERR_MPI_INVALID_CHARACTER -0x0006 /**< There is an invalid character in the digit string. */
-#define POLARSSL_ERR_MPI_BUFFER_TOO_SMALL -0x0008 /**< The output buffer is too small to write too. */
+#define POLARSSL_ERR_MPI_BUFFER_TOO_SMALL -0x0008 /**< The buffer is too small to write too. */
#define POLARSSL_ERR_MPI_NEGATIVE_VALUE -0x000A /**< The input arguments are negative or result in illegal output. */
#define POLARSSL_ERR_MPI_DIVISION_BY_ZERO -0x000C /**< The input argument for division is zero, which is not allowed. */
#define POLARSSL_ERR_MPI_NOT_ACCEPTABLE -0x000E /**< The input arguments are not acceptable. */
@@ -66,6 +66,16 @@
#define POLARSSL_MPI_MAX_SIZE 512 /**< Maximum number of bytes for usable MPIs. */
#define POLARSSL_MPI_MAX_BITS ( 8 * POLARSSL_MPI_MAX_SIZE ) /**< Maximum number of bits for usable MPIs. */
+/*
+ * When reading from files with mpi_read_file() the buffer should have space
+ * for a (short) label, the MPI (in the provided radix), the newline
+ * characters and the '\0'.
+ *
+ * By default we assume at least a 10 char label, a minimum radix of 10
+ * (decimal) and a maximum of 4096 bit numbers (1234 decimal chars).
+ */
+#define POLARSSL_MPI_READ_BUFFER_SIZE 1250
+
/*
* Define the base integer type, architecture-wise
*/
@@ -223,7 +233,7 @@ size_t mpi_size( const mpi *X );
* \param radix Input numeric base
* \param s Null-terminated string buffer
*
- * \return 0 if successful, or an POLARSSL_ERR_MPI_XXX error code
+ * \return 0 if successful, or a POLARSSL_ERR_MPI_XXX error code
*/
int mpi_read_string( mpi *X, int radix, const char *s );
@@ -235,7 +245,7 @@ int mpi_read_string( mpi *X, int radix, const char *s );
* \param s String buffer
* \param slen String buffer size
*
- * \return 0 if successful, or an POLARSSL_ERR_MPI_XXX error code.
+ * \return 0 if successful, or a POLARSSL_ERR_MPI_XXX error code.
* *slen is always updated to reflect the amount
* of data that has (or would have) been written.
*
@@ -251,7 +261,9 @@ int mpi_write_string( const mpi *X, int radix, char *s, size_t *slen );
* \param radix Input numeric base
* \param fin Input file handle
*
- * \return 0 if successful, or an POLARSSL_ERR_MPI_XXX error code
+ * \return 0 if successful, POLARSSL_ERR_MPI_BUFFER_TOO_SMALL if
+ * the file read buffer is too small or a
+ * POLARSSL_ERR_MPI_XXX error code
*/
int mpi_read_file( mpi *X, int radix, FILE *fin );
@@ -263,7 +275,7 @@ int mpi_read_file( mpi *X, int radix, FILE *fin );
* \param radix Output numeric base
* \param fout Output file handle (can be NULL)
*
- * \return 0 if successful, or an POLARSSL_ERR_MPI_XXX error code
+ * \return 0 if successful, or a POLARSSL_ERR_MPI_XXX error code
*
* \note Set fout == NULL to print X on the console.
*/
diff --git a/library/bignum.c b/library/bignum.c
index 6591b5b67..59209563c 100644
--- a/library/bignum.c
+++ b/library/bignum.c
@@ -441,15 +441,19 @@ int mpi_read_file( mpi *X, int radix, FILE *fin )
size_t slen;
char *p;
/*
- * Buffer should have space for (short) label and hexified MPI and '\0'
+ * Buffer should have space for (short) label and decimal formatted MPI,
+ * newline characters and '\0'
*/
- char s[ 2 * POLARSSL_MPI_MAX_SIZE + 10 ];
+ char s[ POLARSSL_MPI_READ_BUFFER_SIZE ];
memset( s, 0, sizeof( s ) );
if( fgets( s, sizeof( s ) - 1, fin ) == NULL )
return( POLARSSL_ERR_MPI_FILE_IO_ERROR );
slen = strlen( s );
+ if( slen == sizeof( s ) - 2 )
+ return( POLARSSL_ERR_MPI_BUFFER_TOO_SMALL );
+
if( s[slen - 1] == '\n' ) { slen--; s[slen] = '\0'; }
if( s[slen - 1] == '\r' ) { slen--; s[slen] = '\0'; }
diff --git a/tests/data_files/mpi_too_big b/tests/data_files/mpi_too_big
new file mode 100644
index 000000000..2d98757fb
--- /dev/null
+++ b/tests/data_files/mpi_too_big
@@ -0,0 +1 @@
+64380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157314759639015364380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157314759639015364380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157314759639015364380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157314759639015364380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157314759639015364380800680355443923012985496149269915138610753401343291807343952413826484237063006136971539473913409092293733259038472039713333596954925632262097903668663321390395296617510709676918001764616185157
diff --git a/tests/suites/test_suite_mpi.data b/tests/suites/test_suite_mpi.data
index ca98f0ce8..0a8c10332 100644
--- a/tests/suites/test_suite_mpi.data
+++ b/tests/suites/test_suite_mpi.data
@@ -61,6 +61,10 @@ Test mpi_read_file #2 (Illegal input)
depends_on:POLARSSL_FS_IO
mpi_read_file:10:"data_files/hash_file_3":"":0
+Test mpi_read_file #3 (Input too big)
+depends_on:POLARSSL_FS_IO
+mpi_read_file:10:"data_files/mpi_too_big":"":POLARSSL_ERR_MPI_BUFFER_TOO_SMALL
+
Base test mpi_write_file #1
depends_on:POLARSSL_FS_IO
mpi_write_file:10:"56125680981752282334141896320372489490613963693556392520816017892111350604111697682705498319512049040516698827829292076808006940873974979584527073481012636016353913462376755556720019831187364993587901952757307830896531678727717924":16:"data_files/mpi_write":0