[xiph-commits] r3867 - liboggz/trunk/src/liboggz
conrad at svn.annodex.net
conrad at svn.annodex.net
Wed Feb 25 21:22:37 PST 2009
Author: conrad
Date: 2009-02-25 21:22:37 -0800 (Wed, 25 Feb 2009)
New Revision: 3867
Modified:
liboggz/trunk/src/liboggz/oggz_auto.c
liboggz/trunk/src/liboggz/oggz_comments.c
Log:
Check for integer overflows in calculations for realloc and
when using strlen returns.
Fixes liboggz portion of Mozilla bug 480014
(Note that issues related to other files in liboggz were already
fixed in earlier commits from an earlier audit, which were
noted for Mozilla bug 468280)
Modified: liboggz/trunk/src/liboggz/oggz_auto.c
===================================================================
--- liboggz/trunk/src/liboggz/oggz_auto.c 2009-02-26 05:22:29 UTC (rev 3866)
+++ liboggz/trunk/src/liboggz/oggz_auto.c 2009-02-26 05:22:37 UTC (rev 3867)
@@ -745,6 +745,7 @@
int size_check;
int *mode_size_ptr;
int i;
+ size_t size_realloc_bytes;
/*
* This is the format of the mode data at the end of the packet for all
@@ -845,11 +846,12 @@
}
#endif
- /*
- * store mode size information in our info struct
- */
- info = realloc(stream->calculate_data,
- sizeof(auto_calc_vorbis_info_t) + (size - 1) * sizeof(int));
+ /* Check that size to be realloc'd doesn't overflow */
+ size_realloc_bytes = sizeof(auto_calc_vorbis_info_t) + (size - 1) * sizeof(int);
+ if (size_realloc_bytes < sizeof (auto_calc_vorbis_info_t)) return -1;
+
+ /* Store mode size information in our info struct */
+ info = realloc(stream->calculate_data, size_realloc_bytes);
if (info == NULL) return -1;
stream->calculate_data = info;
Modified: liboggz/trunk/src/liboggz/oggz_comments.c
===================================================================
--- liboggz/trunk/src/liboggz/oggz_comments.c 2009-02-26 05:22:29 UTC (rev 3866)
+++ liboggz/trunk/src/liboggz/oggz_comments.c 2009-02-26 05:22:37 UTC (rev 3867)
@@ -35,10 +35,13 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <limits.h> /* ULONG_MAX */
#ifndef WIN32
#include <strings.h>
#endif
+#include <assert.h>
+
#include "oggz_private.h"
#include "oggz_vector.h"
@@ -51,12 +54,24 @@
#endif
+/* Ensure comment vector length can be expressed in 32 bits */
+static unsigned long
+oggz_comment_len (const char * s)
+{
+ size_t len;
+
+ if (s == NULL) return 0;
+
+ len = strlen (s);
+ return (unsigned long) MIN(len, 0xFFFFFFFF);
+}
+
static char *
oggz_strdup (const char * s)
{
char * ret;
if (s == NULL) return NULL;
- ret = oggz_malloc (strlen(s) + 1);
+ ret = oggz_malloc (oggz_comment_len(s) + 1);
if (ret == NULL) return NULL;
return strcpy (ret, s);
@@ -583,6 +598,26 @@
return 0;
}
+/*
+ * Pre-condition: at least one of accum, delta are non-zero,
+ * ie. don't call accum_length (0, 0);
+ * \retval 0 Failure: integer overflow
+ */
+static unsigned long
+accum_length (unsigned long * accum, unsigned long delta)
+{
+ /* Pre-condition: don't call accum_length (0, 0) */
+ assert (*accum != 0 || delta != 0);
+
+ /* Check for integer overflow */
+ if (delta > ULONG_MAX - (*accum))
+ return 0;
+
+ *accum += delta;
+
+ return *accum;
+}
+
long
oggz_comments_encode (OGGZ * oggz, long serialno,
unsigned char * buf, long length)
@@ -590,16 +625,20 @@
oggz_stream_t * stream;
char * c = (char *)buf;
const OggzComment * comment;
- int nb_fields = 0, vendor_length = 0, field_length;
- long actual_length, remaining = length;
+ int nb_fields = 0, vendor_length = 0;
+ unsigned long actual_length = 0, remaining = length, field_length;
+ /* Deal with sign of length first */
+ if (length < 0) return 0;
+
stream = oggz_get_stream (oggz, serialno);
if (stream == NULL) return OGGZ_ERR_BAD_SERIALNO;
/* Vendor string */
if (stream->vendor)
- vendor_length = strlen (stream->vendor);
- actual_length = 4 + vendor_length;
+ vendor_length = oggz_comment_len (stream->vendor);
+ if (accum_length (&actual_length, 4 + vendor_length) == 0)
+ return 0;
#ifdef DEBUG
printf ("oggz_comments_encode: vendor = %s\n", stream->vendor);
#endif
@@ -609,9 +648,14 @@
for (comment = oggz_comment_first (oggz, serialno); comment;
comment = oggz_comment_next (oggz, serialno, comment)) {
- actual_length += 4 + strlen (comment->name); /* [size]"name" */
- if (comment->value)
- actual_length += 1 + strlen (comment->value); /* "=value" */
+ /* [size]"name" */
+ if (accum_length (&actual_length, 4 + oggz_comment_len (comment->name)) == 0)
+ return 0;
+ if (comment->value) {
+ /* "=value" */
+ if (accum_length (&actual_length, 1 + oggz_comment_len (comment->value)) == 0)
+ return 0;
+ }
#ifdef DEBUG
printf ("oggz_comments_encode: %s = %s\n",
@@ -621,8 +665,12 @@
nb_fields++;
}
- actual_length++; /* framing bit */
+ /* framing bit */
+ if (accum_length (&actual_length, 1) == 0)
+ return 0;
+ /* NB. actual_length is not modified from here onwards */
+
if (buf == NULL) return actual_length;
remaining -= 4;
@@ -631,7 +679,7 @@
c += 4;
if (stream->vendor) {
- field_length = strlen (stream->vendor);
+ field_length = oggz_comment_len (stream->vendor);
memcpy (c, stream->vendor, MIN (field_length, remaining));
c += field_length; remaining -= field_length;
if (remaining <= 0 ) return actual_length;
@@ -645,16 +693,16 @@
for (comment = oggz_comment_first (oggz, serialno); comment;
comment = oggz_comment_next (oggz, serialno, comment)) {
- field_length = strlen (comment->name); /* [size]"name" */
+ field_length = oggz_comment_len (comment->name); /* [size]"name" */
if (comment->value)
- field_length += 1 + strlen (comment->value); /* "=value" */
+ field_length += 1 + oggz_comment_len (comment->value); /* "=value" */
remaining -= 4;
if (remaining <= 0) return actual_length;
writeint (c, 0, field_length);
c += 4;
- field_length = strlen (comment->name);
+ field_length = oggz_comment_len (comment->name);
memcpy (c, comment->name, MIN (field_length, remaining));
c += field_length; remaining -= field_length;
if (remaining <= 0) return actual_length;
@@ -665,7 +713,7 @@
*c = '=';
c++;
- field_length = strlen (comment->value);
+ field_length = oggz_comment_len (comment->value);
memcpy (c, comment->value, MIN (field_length, remaining));
c += field_length; remaining -= field_length;
if (remaining <= 0) return actual_length;
More information about the commits
mailing list