From e430ecb6073bd5b74fa50d988f558f143524ec3d Mon Sep 17 00:00:00 2001 From: Michael Giacomelli Date: Wed, 30 Sep 2026 09:29:02 -0400 Subject: [PATCH] imageviewer/jpegp: don't crash or hang on damaged files On colour targets the image viewer hands every file its own decoder rejects to jpegp, including damaged ones, but jpegp barely checks its input. Corrupt and truncated files crashed or hung it: - At the end of the file GETC() kept returning stale bytes, so marker searches and table reads never ended. Feed EOI markers (FF D9) instead, which ends every loop, and stop calling read() there. This state is reset in OPEN(): the overlay loader does not clear .bss. - A file ending before any scan decoded as a blank image. Report it as corrupt instead. - Out of range header values were used as array indexes: Huffman and conditioning table IDs, Huffman table sizes, sampling factors, scan component counts and spectral selection. Reject them, and frames of zero width or with no components. - Invalid Huffman codes walked past the code length table, run lengths wrote past coefficient 63, and huge coefficients indexed past the IDCT clamp table. Bound all three. - An odd DAC segment length never ended its loop. - The coefficient buffer size could overflow an int. Co-Authored-By: Claude Opus 5.5 Change-Id: I49465f38d283e159274d2f381029280cea42a8f1 --- apps/plugins/imageviewer/jpegp/BUFFILEGETC.c | 19 ++++++++++-- apps/plugins/imageviewer/jpegp/idct.c | 5 +++- apps/plugins/imageviewer/jpegp/jpeg81.c | 31 +++++++++++++++++--- apps/plugins/imageviewer/jpegp/jpeg81.h | 2 ++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/apps/plugins/imageviewer/jpegp/BUFFILEGETC.c b/apps/plugins/imageviewer/jpegp/BUFFILEGETC.c index a833851b1b..47a5087ef0 100644 --- a/apps/plugins/imageviewer/jpegp/BUFFILEGETC.c +++ b/apps/plugins/imageviewer/jpegp/BUFFILEGETC.c @@ -9,14 +9,25 @@ static unsigned char buff[256]; //TODO: Adjust it... static int length = 0; static int cur_buff_pos = 0; static int file_pos = 0; +/* Past the end of the file. Overlays are loaded without clearing their + .bss, so these are set in OPEN() rather than relied on to start at 0. */ +static bool at_eof; /* read() found the end: don't call it again */ +static unsigned eof_bytes; /* bytes fed since: alternate FF, D9 */ extern int GETC(void) { if (cur_buff_pos >= length) { - length = rb->read(fd, buff, sizeof(buff)); - file_pos += length; + length = at_eof ? 0 : rb->read(fd, buff, sizeof(buff)); cur_buff_pos = 0; + if (length <= 0) + { /* past the end of a damaged file: feed EOI markers (FF D9) + so every loop in the decoder ends instead of spinning */ + at_eof = true; + length = 0; + return (eof_bytes++ & 1) ? 0xD9 : 0xFF; + } + file_pos += length; } return buff[cur_buff_pos++]; @@ -56,11 +67,13 @@ extern void SEEK(int d) } file_pos = rb->lseek(fd, (cur_buff_pos - length) + d, SEEK_CUR); cur_buff_pos = length = 0; + at_eof = false; } extern void POS(int d) { cur_buff_pos = length = 0; + at_eof = false; file_pos = d; rb->lseek(fd, d, SEEK_SET); } @@ -77,6 +90,8 @@ extern void *OPEN(char *f) memset(buff, 0, sizeof(buff)); printf("Opening %s\n", f); cur_buff_pos = length = file_pos = 0; + at_eof = false; + eof_bytes = 0; fd = rb->open(f,O_RDONLY); if ( fd < 0 ) diff --git a/apps/plugins/imageviewer/jpegp/idct.c b/apps/plugins/imageviewer/jpegp/idct.c index 7db1658546..659d4650e6 100644 --- a/apps/plugins/imageviewer/jpegp/idct.c +++ b/apps/plugins/imageviewer/jpegp/idct.c @@ -129,6 +129,9 @@ extern void idct_sq(short *coef, int *sq) for (i=0; i<8; i++) idct1(R+i*8, C+i); for (i=0; i<8; i++) idct1(C+i*8, R+i); - for (i=0; i<64; i++) coef[i] = CLIP[ R[i] >> 15 ]; + for (i=0; i<64; i++) { // clamp: corrupt data can exceed the CLIP table + int v= R[i] >> 15; + coef[i] = CLIP[ v < -256 ? -256 : v > 511 ? 511 : v ]; + } } diff --git a/apps/plugins/imageviewer/jpegp/jpeg81.c b/apps/plugins/imageviewer/jpegp/jpeg81.c index b4849da6f7..05addec626 100644 --- a/apps/plugins/imageviewer/jpegp/jpeg81.c +++ b/apps/plugins/imageviewer/jpegp/jpeg81.c @@ -148,8 +148,11 @@ static int ReadDiff(struct JPEGD *j, int s) // JPEG magnitude stuff. One way to static int ReadHuffmanCode(struct JPEGD *j, int *pb) // index into the sym-table { - int v= GetBit(j); - while ( v >= *pb ) v= 2*v + GetBit(j) - *pb++; + int v= GetBit(j), n= 16; + while ( v >= *pb ) { + if (!--n) return 0; // no code of up to 16 bits: corrupt data + v= 2*v + GetBit(j) - *pb++; + } return v; } @@ -180,6 +183,7 @@ static void ac_decode_huff(struct JPEGD *j, struct COMP *sc, TCOEF *coef) }//else ZRL } k+=r; + if (k > j->Se) return; // corrupt data coef[k]= s; if (k==j->Se) return; } @@ -207,7 +211,8 @@ static void ac_succ_huff(struct JPEGD *j, struct COMP *sc, TCOEF *coef) break; }//else ZRL } - for (; ;k++) if (!ac_refine(j, coef+k)) if (!r--) break; + for (; k <= j->Se; k++) if (!ac_refine(j, coef+k)) if (!r--) break; + if (k > j->Se) return; // corrupt data coef[k]= s; if (k==j->Se) return; } @@ -222,6 +227,7 @@ static void du_sequential_huff(struct JPEGD *j, struct COMP *sc, TCOEF *coef) dc_decode_huff(j, sc, coef); for (k=1; (s=sc->ACS[ReadHuffmanCode(j, sc->ACB)]); k++) { // EOB? k+= s>>4; + if (k > 63) return; // corrupt data if (s==0xf0) continue; // ZRL coef[k]= ReadDiff(j, s&15); if (k==63) return; @@ -635,6 +641,8 @@ static int set_dim(struct JPEGD *j, int d) // d= 1 (LL) or 8 (DCT) C->Hi= C->Vi>>4; C->Vi&= 15; C->Qi= GETC(); + if (C->Hi < 1 || C->Hi > 4 || C->Vi < 1 || C->Vi > 4 || C->Qi > 3) + return -1; // corrupt if ( C->Hi > j->Hmax ) j->Hmax = C->Hi; if ( C->Vi > j->Vmax ) j->Vmax = C->Vi; @@ -709,12 +717,13 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) int La= GETWbi(); printf("DAC\n"); printf(" Arithmetic Conditioning\n parameters:\n"); - for (La-=2; La; La-=2) + for (La-=2; La > 1; La-=2) { int CB= GETC(); int Tc= CB>>4; int Tb= CB&15; int Cs= GETC(); + if (Tb > 3) return JPEGENUMERR_CORRUPT; if (Tc) // AC { printf(" AC%d Kx=%d\n", Tb, Cs); @@ -744,6 +753,7 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) int CH= GETC(); int Tc= CH>>4; int Th= CH&15; + if (Tc > 1 || Th > 3) return JPEGENUMERR_CORRUPT; int *B= j->HTB[Tc][Th]; unsigned char *S= j->HTS[Tc][Th]; printf(" %s%d\n", Tc?"AC":"DC", Th); @@ -755,6 +765,7 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) B[i]= N; // running total } printf("\n"); + if (N > 256 || 17 + N > Lh) return JPEGENUMERR_CORRUPT; printf(" S: %d symbol bytes\n", N); for (i=0; iNf>4) return JPEGENUMERR_COMP4; + if (!j->Nf) return JPEGENUMERR_CORRUPT; if (!j->Y) return JPEGENUMERR_ZEROY; // I have no idea about this DNL stuff + if (!j->X) return JPEGENUMERR_CORRUPT; j->SOF= marker; if ( (j->SOF&3)==3 ) // LOSSLESS-mode { int TotalDU= set_dim(j, 1); // for malloc: in samples as coeff; + if (TotalDU < 0) return JPEGENUMERR_CORRUPT; + if (TotalDU > 0x7fffffff / (int)sizeof(DU)) return JPEGENUMERR_MALLOC; if (j->SOF > 0xC8) { // arithmetic: @@ -819,6 +834,8 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) else // DCT-mode { int TotalDU= set_dim(j, 8); // for malloc in DU; + if (TotalDU < 0) return JPEGENUMERR_CORRUPT; + if (TotalDU > 0x7fffffff / (int)sizeof(DU)) return JPEGENUMERR_MALLOC; printf(" %d MCU (%d x %d)\n", j->mcu_total, j->mcu_width, j->mcu_height); @@ -857,6 +874,7 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) else if ( marker == 0xD9 ) // EOI { printf("EOI\n"); + if (!j->scans) return JPEGENUMERR_CORRUPT; // no image data return JPEGENUM_OK; } else if ( marker == 0xDA ) // SOS @@ -865,6 +883,8 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) GETWbi(); //Ls printf("SOS\n"); j->Ns= GETC();//Ns + j->scans++; + if (j->Ns < 1 || j->Ns > j->Nf) return JPEGENUMERR_CORRUPT; printf(" Ns: %d (%s scan)\n", j->Ns, (j->Ns>1)?"Interleaved":"Single"); for (ci=0; ciNs; ci++) @@ -874,6 +894,7 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) int T= GETC(); int Td= T>>4; int Ta= T&15; + if (Td > 3 || Ta > 3) return JPEGENUMERR_CORRUPT; printf(" Cs=%d Td=%d Ta=%d\n", Cs, Td, Ta); {// safe search @@ -908,6 +929,8 @@ extern enum JPEGENUM JPEGDecode(struct JPEGD *j) j->Ah= j->Al>>4; j->Al&= 15; j->Al2= 1<Al;//pre-computed + if (j->Se > 63 || j->Ss > j->Se || j->Al > 13) + return JPEGENUMERR_CORRUPT; printf(" %s: %d\n", ((j->SOF&3)==3)?"Px":"Ss", j->Ss); printf(" Se: %d\n", j->Se); diff --git a/apps/plugins/imageviewer/jpegp/jpeg81.h b/apps/plugins/imageviewer/jpegp/jpeg81.h index 8d9c2a04a3..a5b23abf3a 100644 --- a/apps/plugins/imageviewer/jpegp/jpeg81.h +++ b/apps/plugins/imageviewer/jpegp/jpeg81.h @@ -32,6 +32,7 @@ enum JPEGENUM { JPEGENUMERR_MARKERDNL, // DNL marker found (not supported) JPEGENUMERR_ZEROY, // Y in SOFn is zero (DNL?) JPEGENUMERR_COMPNOTFOUND, // Scan component selector (Csj) not found among Component identifiers (Ci) + JPEGENUMERR_CORRUPT, // a header value out of range }; #include @@ -105,6 +106,7 @@ struct JPEGD { // The JPEG DECODER OBJECT void *jpeg_mem; // <-- free me int Hmax, Vmax; // for conversion + int scans; // scans decoded: none means no image data bool jfif; // saw a JFIF APP0 marker unsigned char adobe; // Adobe APP14 transform flag + 1, 0 if none int mcu_width;