Commit bc2a749

committed

exif/heic: Avoid undefined behaviour in address calculations

It is illegal to construct out-of-bound pointers, even if they are not dereferenced. The current bound checks rely on undefined behaviour. Fix this by introducing convenience macros that check the remaining length.

1 parent b90ab81 commit bc2a749Copy full SHA for bc2a749

File tree

1 file changed

+35

-37

lines changed

ext/exif
- exif.c

1 file changed

+35

-37

lines changed

`‎ext/exif/exif.c‎`

Lines changed: 35 additions & 37 deletions

Original file line number	Diff line number	Diff line change
`@@ -4310,64 +4310,59 @@ static void exif_isobmff_parse_meta(unsigned char data, unsigned char end, iso`
`4310`	`4310`	`unsigned char box_offset, p, *p2;`
`4311`	`4311`	`int header_size, exif_id = -1, version, item_count, i;`
`4312`	`4312`
`4313`		`- for (box_offset = data + 4; box_offset + 16 < end; box_offset += box.size) {`
	`4313`	`+ size_t remain;`
	`4314`	`+#define CHECK(n) do { \`
	`4315`	`+ if (remain < n) { \`
	`4316`	`+ return; \`
	`4317`	`+ } \`
	`4318`	`+} while (0)`
	`4319`	`+#define ADVANCE(n) do { \`
	`4320`	`+ CHECK(n); \`
	`4321`	`+ remain -= n; \`
	`4322`	`+ p += n; \`
	`4323`	`+} while (0)`
	`4324`	`+`
	`4325`	`+ for (box_offset = data + 4; box_offset < end - 16; box_offset += box.size) {`
`4314`	`4326`	`header_size = exif_isobmff_parse_box(box_offset, &box);`
	`4327`	`+ p = box_offset;`
	`4328`	`+ remain = end - p;`
	`4329`	`+`
`4315`	`4330`	`if (box.type == FOURCC("iinf")) {`
`4316`		`- p = box_offset + header_size;`
`4317`		`- if (p >= end) {`
`4318`		`- return;`
`4319`		`- }`
	`4331`	`+ ADVANCE(header_size);`
`4320`	`4332`	`version = p[0];`
`4321`		`- p+=4;`
	`4333`	`+ ADVANCE(4);`
`4322`	`4334`	`if (version < 2) {`
`4323`		`- if (p + 2 >= end) {`
`4324`		`- return;`
`4325`		`- }`
`4326`		`- item_count = php_ifd_get16u(p, 1);`
`4327`		`- p += 2;`
	`4335`	`+ ADVANCE(2);`
	`4336`	`+ item_count = php_ifd_get16u(p - 2, 1);`
`4328`	`4337`	`} else {`
`4329`		`- if (p + 4 >= end) {`
`4330`		`- return;`
`4331`		`- }`
`4332`		`- item_count = php_ifd_get32u(p, 1);`
`4333`		`- p += 4;`
	`4338`	`+ ADVANCE(4);`
	`4339`	`+ item_count = php_ifd_get32u(p - 4, 1);`
`4334`	`4340`	`}`
`4335`		`- for (i = 0; i < item_count && p +20< end; i++) {`
	`4341`	`+ for (i = 0; i < item_count && p < end-20; i++) {`
`4336`	`4342`	`header_size = exif_isobmff_parse_box(p, &item);`
`4337`		`- if (p + header_size + 12 >= end) {`
`4338`		`- return;`
`4339`		`- }`
	`4343`	`+ CHECK(header_size + 12);`
`4340`	`4344`	`if (!memcmp(p + header_size + 8, "Exif", 4)) {`
`4341`	`4345`	`exif_id = php_ifd_get16u(p + header_size + 4, 1);`
`4342`	`4346`	`break;`
`4343`	`4347`	`}`
`4344`		`- p+=item.size;`
	`4348`	`+ ADVANCE(item.size);`
`4345`	`4349`	`}`
`4346`	`4350`	`if (exif_id < 0) {`
`4347`	`4351`	`break;`
`4348`	`4352`	`}`
`4349`	`4353`	`}`
`4350`	`4354`	`else if (box.type == FOURCC("iloc")) {`
`4351`		`- p = box_offset + header_size;`
`4352`		`- if (p >= end) {`
`4353`		`- return;`
`4354`		`- }`
	`4355`	`+ ADVANCE(header_size);`
`4355`	`4356`	`version = p[0];`
`4356`		`- p+=6;`
	`4357`	`+ ADVANCE(6);`
`4357`	`4358`	`if (version < 2) {`
`4358`		`- if (p + 2 >= end) {`
`4359`		`- return;`
`4360`		`- }`
`4361`		`- item_count = php_ifd_get16u(p, 1);`
`4362`		`- p += 2;`
	`4359`	`+ ADVANCE(2);`
	`4360`	`+ item_count = php_ifd_get16u(p - 2, 1);`
`4363`	`4361`	`} else {`
`4364`		`- if (p + 4 >= end) {`
`4365`		`- return;`
`4366`		`- }`
`4367`		`- item_count = php_ifd_get32u(p, 1);`
`4368`		`- p += 4;`
	`4362`	`+ ADVANCE(4);`
	`4363`	`+ item_count = php_ifd_get32u(p - 4, 1);`
`4369`	`4364`	`}`
`4370`		`- for (i = 0, p2 = p; i < item_count && p +16< end; i++, p2 += 16) {`
	`4365`	`+ for (i = 0, p2 = p; i < item_count && p < end-16; i++, p2 += 16) {`
`4371`	`4366`	`if (php_ifd_get16u(p2, 1) == exif_id) {`
`4372`	`4367`	`pos->offset = php_ifd_get32u(p2 + 8, 1);`
`4373`	`4368`	`pos->size = php_ifd_get32u(p2 + 12, 1);`
`@@ -4377,6 +4372,9 @@ static void exif_isobmff_parse_meta(unsigned char data, unsigned char end, iso`
`4377`	`4372`	`break;`
`4378`	`4373`	`}`
`4379`	`4374`	`}`
	`4375`	`+`
	`4376`	`+#undef ADVANCE`
	`4377`	`+#undef CHECK`
`4380`	`4378`	`}`
`4381`	`4379`
`4382`	`4380`	`static bool exif_scan_HEIF_header(image_info_type ImageInfo, unsigned char buf)`

0 commit comments

Comments

(0)

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Commit bc2a749

File tree

1 file changed

1 file changed

`‎ext/exif/exif.c‎`

0 commit comments