| Message ID | 1431686162-12479-1-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 05/15/2015 03:36 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Coverty Scan is warning that malicious or malformed tables being read in > from file could cause problems with the low 32 bit allocator. Add some > stricter checking of the size of the buffer being read and also the total > ACPI table size. ACPI stipulates that the table size is a 32 bit integer > (which is huge), but we need to keep to that maximum upper limit to be > in spec. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index 81fb096..cca1135 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -667,6 +667,10 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) > } > continue; > } > + if (n > (ssize_t)sizeof(buffer)) > + goto too_big; /* Unlikely */ > + if (size + n > 0xffffffff) > + goto too_big; /* Very unlikely */ > > if ((tmp = (uint8_t*)fwts_low_realloc(ptr, size + n + 1)) == NULL) { > free(ptr); > @@ -678,6 +682,11 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) > } > *length = size; > return ptr; > + > +too_big: > + free(ptr); > + *length = 0; > + return NULL; > } > > /* > Acked-by: Alex Hung <alex.hung@canonical.com>
On 2015年05月15日 18:36, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Coverty Scan is warning that malicious or malformed tables being read in > from file could cause problems with the low 32 bit allocator. Add some > stricter checking of the size of the buffer being read and also the total > ACPI table size. ACPI stipulates that the table size is a 32 bit integer > (which is huge), but we need to keep to that maximum upper limit to be > in spec. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_acpi_tables.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c > index 81fb096..cca1135 100644 > --- a/src/lib/src/fwts_acpi_tables.c > +++ b/src/lib/src/fwts_acpi_tables.c > @@ -667,6 +667,10 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) > } > continue; > } > + if (n > (ssize_t)sizeof(buffer)) > + goto too_big; /* Unlikely */ > + if (size + n > 0xffffffff) > + goto too_big; /* Very unlikely */ > > if ((tmp = (uint8_t*)fwts_low_realloc(ptr, size + n + 1)) == NULL) { > free(ptr); > @@ -678,6 +682,11 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) > } > *length = size; > return ptr; > + > +too_big: > + free(ptr); > + *length = 0; > + return NULL; > } > > /* Acked-by: Ivan Hu<ivan.hu@canonical.com>
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c index 81fb096..cca1135 100644 --- a/src/lib/src/fwts_acpi_tables.c +++ b/src/lib/src/fwts_acpi_tables.c @@ -667,6 +667,10 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) } continue; } + if (n > (ssize_t)sizeof(buffer)) + goto too_big; /* Unlikely */ + if (size + n > 0xffffffff) + goto too_big; /* Very unlikely */ if ((tmp = (uint8_t*)fwts_low_realloc(ptr, size + n + 1)) == NULL) { free(ptr); @@ -678,6 +682,11 @@ static uint8_t *fwts_acpi_load_table_from_file(const int fd, size_t *length) } *length = size; return ptr; + +too_big: + free(ptr); + *length = 0; + return NULL; } /*