[PATCH 0/2] v4l-utils: Fix crashes found by Mayhem

[PATCH 0/2] v4l-utils: Fix crashes found by Mayhem

[Gregor Jasny]

The Mayhem Team ran their code checker over the Debian archive and
also found two crashes in v4l-utils.

See http://lists.debian.org/debian-devel/2013/06/msg00720.html

Gregor Jasny (2):
  libv4lconvert: Prevent integer overflow by checking width and height
  keytable: Always check if strtok return value is null

 lib/libv4lconvert/ov511-decomp.c |  7 ++++++-
 lib/libv4lconvert/ov518-decomp.c |  7 ++++++-
 utils/keytable/keytable.c        | 19 ++++++++++++++++---
 3 files changed, 28 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH 1/2] libv4lconvert: Prevent integer overflow by checking width and height

[Gregor Jasny]

The Mayhem Team found a crash caused by an integer overflow.
Details are here:
http://www.forallsecure.com/bug-reports/8aae67d864bce76993f3f9812b4a2aeea0eb38da/

Signed-off-by: Gregor Jasny <gjasny@googlemail.com>
---
 lib/libv4lconvert/ov511-decomp.c | 7 ++++++-
 lib/libv4lconvert/ov518-decomp.c | 7 ++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/lib/libv4lconvert/ov511-decomp.c b/lib/libv4lconvert/ov511-decomp.c
index 90fc4b1..971d497 100644
--- a/lib/libv4lconvert/ov511-decomp.c
+++ b/lib/libv4lconvert/ov511-decomp.c
@@ -14,6 +14,7 @@
  * Free Software Foundation; version 2 of the License.
  */
 
+#include <limits.h>
 #include <string.h>
 #include <unistd.h>
 #include "helper-funcs.h"
@@ -640,7 +641,11 @@ int main(int argc, char *argv[])
 
 
 		dest_size = width * height * 3 / 2;
-		if (dest_size > sizeof(dest_buf)) {
+		if (width <= 0 || width > SHRT_MAX || height <= 0 || height > SHRT_MAX) {
+			fprintf(stderr, "%s: error: width or height out of bounds\n",
+					argv[0]);
+			dest_size = -1;
+		} else if (dest_size > sizeof(dest_buf)) {
 			fprintf(stderr, "%s: error: dest_buf too small, need: %d\n",
 					argv[0], dest_size);
 			dest_size = -1;
diff --git a/lib/libv4lconvert/ov518-decomp.c b/lib/libv4lconvert/ov518-decomp.c
index 47b5cbb..91d908c 100644
--- a/lib/libv4lconvert/ov518-decomp.c
+++ b/lib/libv4lconvert/ov518-decomp.c
@@ -15,6 +15,7 @@
  * Free Software Foundation; version 2 of the License.
  */
 
+#include <limits.h>
 #include <string.h>
 #include <unistd.h>
 #include "helper-funcs.h"
@@ -1454,7 +1455,11 @@ int main(int argc, char *argv[])
 
 
 		dest_size = width * height * 3 / 2;
-		if (dest_size > sizeof(dest_buf)) {
+		if (width <= 0 || width > SHRT_MAX || height <= 0 || height > SHRT_MAX) {
+			fprintf(stderr, "%s: error: width or height out of bounds\n",
+					argv[0]);
+			dest_size = -1;
+		} else if (dest_size > sizeof(dest_buf)) {
 			fprintf(stderr, "%s: error: dest_buf too small, need: %d\n",
 					argv[0], dest_size);
 			dest_size = -1;
-- 
1.8.3.1

[PATCH 2/2] keytable: Always check if strtok return value is null

[Gregor Jasny]

The Mayhem Team found a crash caused by a nullptr.
Details are here:
http://www.forallsecure.com/bug-reports/567323cd26f180910beb03ae26afb40c432a0c6a/

Signed-off-by: Gregor Jasny <gjasny@googlemail.com>
---
 utils/keytable/keytable.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c
index 06b3d95..8bcd5c4 100644
--- a/utils/keytable/keytable.c
+++ b/utils/keytable/keytable.c
@@ -207,13 +207,19 @@ static error_t parse_keyfile(char *fname, char **table)
 			p++;
 			p = strtok(p, "\n\t =:");
 			do {
+				if (!p)
+					goto err_einval;
 				if (!strcmp(p, "table")) {
 					p = strtok(NULL,"\n, ");
+					if (!p)
+						goto err_einval;
 					*table = malloc(strlen(p) + 1);
 					strcpy(*table, p);
 				} else if (!strcmp(p, "type")) {
 					p = strtok(NULL, " ,\n");
 					do {
+						if (!p)
+							goto err_einval;
 						if (!strcasecmp(p,"rc5") || !strcasecmp(p,"rc-5"))
 							ch_proto |= RC_5;
 						else if (!strcasecmp(p,"rc6") || !strcasecmp(p,"rc-6"))
@@ -447,6 +453,8 @@ static error_t parse_opt(int k, char *arg, struct argp_state *state)
 	case 'p':
 		p = strtok(arg, ",;");
 		do {
+			if (!p)
+				goto err_inval;
 			if (!strcasecmp(p,"rc5") || !strcasecmp(p,"rc-5"))
 				ch_proto |= RC_5;
 			else if (!strcasecmp(p,"rc6") || !strcasecmp(p,"rc-6"))
@@ -813,14 +821,19 @@ static int v1_get_sw_enabled_protocol(char *dirname)
 		return 0;
 	}
 
-	p = strtok(buf, " \n");
-	rc = atoi(p);
-
 	if (fclose(fp)) {
 		perror(name);
 		return errno;
 	}
 
+	p = strtok(buf, " \n");
+	if (!p) {
+		fprintf(stderr, "%s has invalid content: '%s'\n", name, buf);
+		return 0;
+	}
+
+	rc = atoi(p);
+
 	if (debug)
 		fprintf(stderr, "protocol %s is %s\n",
 			name, rc? "enabled" : "disabled");
-- 
1.8.3.1

Re: [PATCH 0/2] v4l-utils: Fix crashes found by Mayhem

[Hans de Goede]

Hi,

Thanks for working on this, both patches look good, ack series.

Regards,

Hans


On 06/27/2013 11:11 PM, Gregor Jasny wrote:
> The Mayhem Team ran their code checker over the Debian archive and
> also found two crashes in v4l-utils.
>
> See http://lists.debian.org/debian-devel/2013/06/msg00720.html
>
> Gregor Jasny (2):
>    libv4lconvert: Prevent integer overflow by checking width and height
>    keytable: Always check if strtok return value is null
>
>   lib/libv4lconvert/ov511-decomp.c |  7 ++++++-
>   lib/libv4lconvert/ov518-decomp.c |  7 ++++++-
>   utils/keytable/keytable.c        | 19 ++++++++++++++++---
>   3 files changed, 28 insertions(+), 5 deletions(-)
>