From 259133f0105e22cb5790b8148b96dea071bf3280 Mon Sep 17 00:00:00 2001 From: Larry Gritz Date: Sat, 4 Jul 2026 09:38:48 -0400 Subject: [PATCH] fix(dpx): fix heap overflow in 1-channel 10-bit filled scanline swap Read10bitFilled() unpacks a scanline of 10-bit "filled" (Method A/B) samples backwards, and for 1-channel images applies a work-around that swaps the first and third datum of each group of 3 packed samples (so column order comes out right). The swap didn't check that a full group of 3 remained in bounds: whenever the scanline's datum count isn't a multiple of 3, the last (partial) group has only 1 or 2 real datums, and obuf[count + 2] reaches past the end of the caller-owned scanline buffer -- both reading and writing one sample out of bounds. Guard the swap so it only fires when a complete group of 3 remains. Add a regression test in testsuite/dpx using a crafted 1-channel, 10-bit, "Filled method A" DPX subimage (width 80, not a multiple of 3) that reproduces the overflow under ASan prior to this fix. Assisted-by: Claude Code / Sonnet 5 Signed-off-by: Larry Gritz --- src/dpx.imageio/libdpx/ReaderInternal.h | 8 ++- testsuite/dpx/ref/out.txt | 58 ++++++++++++++++++ testsuite/dpx/run.py | 8 +++ .../src/crash-1chan-10bit-filled-methodA.dpx | Bin 0 -> 40509 bytes 4 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 testsuite/dpx/src/crash-1chan-10bit-filled-methodA.dpx diff --git a/src/dpx.imageio/libdpx/ReaderInternal.h b/src/dpx.imageio/libdpx/ReaderInternal.h index 124f9405a6..74899ab09e 100644 --- a/src/dpx.imageio/libdpx/ReaderInternal.h +++ b/src/dpx.imageio/libdpx/ReaderInternal.h @@ -172,7 +172,8 @@ namespace dpx BUF *obuf = data + bufoff; int index = (block.x1 * sizeof(U32)) % numberOfComponents; - for (int count = (block.x2 - block.x1 + 1) * numberOfComponents - 1; count >= 0; count--) + const int total = (block.x2 - block.x1 + 1) * numberOfComponents; + for (int count = total - 1; count >= 0; count--) { // unpacking the buffer backwards U16 d1 = U16(readBuf[(count + index) / 3] >> ((2 - (count + index) % 3) * 10 + PADDINGBITS) & 0x3ff); @@ -181,7 +182,10 @@ namespace dpx BaseTypeConverter(d1, obuf[count]); // work-around for 1-channel DPX images - to swap the outlying pixels, otherwise the columns are in the wrong order - if (numberOfComponents == 1 && count % 3 == 0) + // Only when a full group of 3 remains in bounds -- a short trailing + // group (when total isn't a multiple of 3) has no third datum to + // swap with, and obuf[count + 2] would be out of bounds. + if (numberOfComponents == 1 && count % 3 == 0 && count + 2 < total) std::swap(obuf[count], obuf[count + 2]); } #endif diff --git a/testsuite/dpx/ref/out.txt b/testsuite/dpx/ref/out.txt index 88458509f1..d4c13b5d2e 100644 --- a/testsuite/dpx/ref/out.txt +++ b/testsuite/dpx/ref/out.txt @@ -161,3 +161,61 @@ PASS oiiotool ERROR: read : "src/crash-badusersize.dpx": Corrupt userbuf: size claims 4160749568 but whole file size is 2054 Full command line was: > oiiotool src/crash-badusersize.dpx -o test.tif +Reading src/crash-1chan-10bit-filled-methodA.dpx +src/crash-1chan-10bit-filled-methodA.dpx : 80 x 60, 3 channel, uint10 dpx + 2 subimages: 80x60 [u10,u10,u10], 80x60 [u10] + subimage 0: 80 x 60, 3 channel, uint10 dpx + SHA-1: 0071FD79957267E8C9978DB3360AF9C752E31556 + channel list: R, G, B + full/display size: 2143289344 x 2143289344 + full/display origin: 0, 0 + ImageDescription: "view angle: left" + Orientation: 1 (normal) + PixelAspectRatio: 1 + dpx:Colorimetric: "User defined" + dpx:DittoKey: 1 + dpx:EndOfImagePadding: 0 + dpx:EndOfLinePadding: 0 + dpx:FramePosition: 0 + dpx:ImageDescriptor: "RGB" + dpx:Packing: "Filled, method A" + dpx:SequenceLength: 0 + dpx:SourceImageFileName: "P" + dpx:TimeCode: "00:00:00:00" + dpx:Transfer: "Undefined" + dpx:UserBits: 0 + dpx:Version: "V2.0" + dpx:VerticalSampleRate: -1.7147e+38 + dpx:XScannedSize: 1.4013e-45 + dpx:YScannedSize: 1.4013e-45 + oiio:BitsPerSample: 10 + oiio:subimages: 2 + smpte:TimeCode: 00:00:00:00 + subimage 1: 80 x 60, 1 channel, uint10 dpx + SHA-1: 26C5096595B63C5BF3AC347712B11353BB55F1EC + channel list: channel0 + full/display size: 2143289344 x 2143289344 + full/display origin: 0, 0 + ImageDescription: "view angle: left" + Orientation: 1 (normal) + PixelAspectRatio: 1 + dpx:Colorimetric: "User defined" + dpx:DittoKey: 1 + dpx:EndOfImagePadding: 0 + dpx:EndOfLinePadding: 0 + dpx:FramePosition: 0 + dpx:HighQuantity: 7.57306e-29 + dpx:ImageDescriptor: "User defined" + dpx:Packing: "Filled, method A" + dpx:SequenceLength: 0 + dpx:SourceImageFileName: "P" + dpx:TimeCode: "00:00:00:00" + dpx:Transfer: "User defined" + dpx:UserBits: 0 + dpx:Version: "V2.0" + dpx:VerticalSampleRate: -1.7147e+38 + dpx:XScannedSize: 1.4013e-45 + dpx:YScannedSize: 1.4013e-45 + oiio:BitsPerSample: 10 + oiio:subimages: 2 + smpte:TimeCode: 00:00:00:00 diff --git a/testsuite/dpx/run.py b/testsuite/dpx/run.py index 536e705927..1acaee89e7 100755 --- a/testsuite/dpx/run.py +++ b/testsuite/dpx/run.py @@ -37,3 +37,11 @@ # Regression tests command += oiiotool("src/crash-badusersize.dpx -o test.tif", failureok=True) + +# Regression test: crafted DPX with a 1-channel, 10-bit, "Filled method A" +# packed subimage whose width (80) is not a multiple of 3. The 1-channel +# work-around in Read10bitFilled() swapped the first and third datum of +# each group of 3 packed samples, but did not check that a full group of 3 +# remained for the last (partial) group in the scanline, writing/reading +# one uint16 past the end of the caller's scanline buffer. +command += info_command("src/crash-1chan-10bit-filled-methodA.dpx", safematch=True) diff --git a/testsuite/dpx/src/crash-1chan-10bit-filled-methodA.dpx b/testsuite/dpx/src/crash-1chan-10bit-filled-methodA.dpx new file mode 100644 index 0000000000000000000000000000000000000000..50579aeab1a2a81d266e67cf9adca88e1c22e4d6 GIT binary patch literal 40509 zcmeI5Urbwd6vuA?8L-Y_Mik>>YShRw><|;93j>Br6tf6}7Fjep5-JqC(adJ-VOv?Y zhb5AT2~!{NPc{gnpkwjH2OJ$5g9Z%`o7n@#X%>UZ1AjK@W})6g_Y0>tzn0rj;1a$! zx%_^=^ZTB2&-uWwZ9+Iym+z@m(iEjC-yl%Ph}WpS9-KI*wTCJs$VY9XpzunyiiLzLWNyjn>A)bt{<~p0!K_ zLh7|lp(SCHlycQtaBnC4Y`<5G`5`4^{urodL|jLljwVaZp*ojir^V%X+av6J6#nd=<%6fv)}2xw8XH6 zbEo>WcOsG`cEm7NEZ5~bf5zHhOMApHk@@P0r$>{>oR%||o?33URleGOZ0ZNs!0f-S zZ*Kqc#93Q~Hm9v80>4u%*)6ogyJkjZ`+9N^F^o1Ymb>Sg{k7_b?#|4Tw8suKn^OA* zGqw+Hzu51&y6+`fEsMn?A3YjbY$MthtL2U3x3BeGNqvOpUS5*^9O7mG(a zJsJ_u=%cI_Zfhx;3(#77(^;OcZ7eyWMD820!4`TnOm66@tQKw?@1Op;IVmGYZxP%# znvYE5_M0|D(;L|?tA*P-+A3Zt2u^%^;MAt)T8op?`}|28EvL-+*Bg4ikk!R~W&32~ zpK)4ur#^hO>h*)8$EJUZrZ=LU>Wp>soKI&}yxiMA^<%qt?oLbL-LdP=u1{{9-0&#Z z&|DTztD&{{j6FUOn#;W$_#@ld@d0s^X<-8a5C8!X009sH0T2KI5C8!X009sH0T2KI z5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X z009sH0T2KI5C8!XSX%`2{N4WAs%T}imGGl~q!iaB{{BBMCj*K_9bz0V{BJz)SJ!8X zDJ5-Fo^G*1ZH_QCUMloIeliv>)g@U}-3B_D1 zJ+uUm%womlSCY-AAA0j>VC?++S}tNO^kU6No84}w9x{1@X^)o|o9?3?T3?Tydr#&= zw&(xp{fad7Y|-n4$D_H-SF2ggHRUJn+=1X!R#kOPtbAhCW8QUZWTa$c#6N+fempWC zvZ-y#viY;3>A1#dbDnd3cjt#~wu;hNIdZ*LM`xCL7jZlFF_QT##m3{W_MF#YX;zXx zZt!)x2h*R5mKWD+HT3hR;>zXr))()8LFTg*n~{2GY3rafHRkL2k~83Qt)B1E>mT(D zq!@YXiz%_Jc6OeX`RKLr_Z6e{(4rc|%{iROId_KoObVOO$VQUVCWKXG-cvCOBc#PeA<$lQooczGY}zSG#_%^{b@5 zsW(4*>ja%OI*;a};6!%)=O^j4aQ#w2