Bug 504888. qcms: Avoid integer overflow when checking buffer bounds. r=bobbyholley
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Tue, 21 Jul 2009 23:32:27 -0400
changeset 30543 8b4c3179b68f58a12408ad1fa205c8b2cb9c6a36
parent 30542 5b0d9f36c0b3e83618130be23b1fc521a8078a77
child 30544 b0f7ccdb99c88d16cc47a490c44b7930fbd3a4d9
push id8124
push userjmuizelaar@mozilla.com
push dateWed, 22 Jul 2009 03:46:00 +0000
treeherdermozilla-central@8b4c3179b68f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbobbyholley
bugs504888
milestone1.9.2a1pre
Bug 504888. qcms: Avoid integer overflow when checking buffer bounds. r=bobbyholley Found by Chris Evans
gfx/qcms/iccread.c
--- a/gfx/qcms/iccread.c
+++ b/gfx/qcms/iccread.c
@@ -75,37 +75,40 @@ struct mem_source
 static void invalid_source(struct mem_source *mem, const char *reason)
 {
 	mem->valid = false;
 	mem->invalid_reason = reason;
 }
 
 static uint32_t read_u32(struct mem_source *mem, size_t offset)
 {
-	if (offset + 4 > mem->size) {
+	/* Subtract from mem->size instead of the more intuitive adding to offset.
+	 * This avoids overflowing offset. The subtraction is safe because
+	 * mem->size is guaranteed to be > 4 */
+	if (offset > mem->size - 4) {
 		invalid_source(mem, "Invalid offset");
 		return 0;
 	} else {
 		return be32_to_cpu(*(__be32*)(mem->buf + offset));
 	}
 }
 
 static uint16_t read_u16(struct mem_source *mem, size_t offset)
 {
-	if (offset + 2 > mem->size) {
+	if (offset > mem->size - 2) {
 		invalid_source(mem, "Invalid offset");
 		return 0;
 	} else {
 		return be16_to_cpu(*(__be16*)(mem->buf + offset));
 	}
 }
 
 static uint8_t read_u8(struct mem_source *mem, size_t offset)
 {
-	if (offset + 1 > mem->size) {
+	if (offset > mem->size - 1) {
 		invalid_source(mem, "Invalid offset");
 		return 0;
 	} else {
 		return *(uint8_t*)(mem->buf + offset);
 	}
 }
 
 static s15Fixed16Number read_s15Fixed16Number(struct mem_source *mem, size_t offset)
@@ -663,24 +666,29 @@ qcms_profile* qcms_profile_from_memory(c
 	struct mem_source source;
 	struct mem_source *src = &source;
 	struct tag_index index;
 	qcms_profile *profile;
 
 	source.buf = mem;
 	source.size = size;
 	source.valid = true;
+
 	length = read_u32(src, 0);
 	if (length <= size) {
 		// shrink the area that we can read if appropriate
 		source.size = length;
 	} else {
 		return INVALID_PROFILE;
 	}
 
+	/* ensure that the profile size is sane so it's easier to reason about */
+	if (source.size <= 64 || source.size >= MAX_PROFILE_SIZE)
+		return INVALID_PROFILE;
+
 	profile = qcms_profile_create();
 	if (!profile)
 		return NO_MEM_PROFILE;
 
 	check_CMM_type_signature(src);
 	check_profile_version(src);
 	read_class_signature(profile, src);
 	read_rendering_intent(profile, src);