Don't traverse linked list when saving a marker

If the calling application invokes jpeg_save_markers() to save a
particular type of marker, then the save_marker() function will be
invoked for every marker of that type that is encountered.  Previously,
only the head of the marker linked list was stored (in
jpeg_decompress_struct), so save_marker() had to traverse the entire
linked list before it could add a new marker to the tail of the list.
That caused CPU usage to grow exponentially with the number of markers.
Referring to #764, it is possible to create a JPEG image that contains
an excessive number of markers.  The specific reproducer that uncovered
this issue is a specially-crafted 1-megabyte malformed JPEG image with
tens of thousands of APP1 markers, which required approximately 30
seconds of CPU time (on a modern Intel processor) to process.  However,
it should also be possible to create a legitimate JPEG image that
reproduces the issue (such as an image with tens of thousands of
duplicate EXIF tags.)

This commit introduces a new pointer (in jpeg_decomp_master, in order to
preserve backward ABI compatibility) that is used to store the tail of
the marker linked list whenever a marker is added to it.  Thus, it is no
longer necessary to traverse the list when adding a marker, and CPU
usage will grow linearly rather than exponentially with the number of
markers.

Fixes #764
This commit is contained in:
DRC
2024-05-14 11:41:16 -04:00
parent 7fa4b5b762
commit 0fc7313e54
3 changed files with 12 additions and 10 deletions

View File

@@ -6,7 +6,7 @@
* Lossless JPEG Modifications:
* Copyright (C) 1999, Ken Murchison.
* libjpeg-turbo Modifications:
* Copyright (C) 2012, 2015, 2022, D. R. Commander.
* Copyright (C) 2012, 2015, 2022, 2024, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
@@ -819,13 +819,11 @@ save_marker(j_decompress_ptr cinfo)
/* Done reading what we want to read */
if (cur_marker != NULL) { /* will be NULL if bogus length word */
/* Add new marker to end of list */
if (cinfo->marker_list == NULL) {
cinfo->marker_list = cur_marker;
if (cinfo->marker_list == NULL || cinfo->master->marker_list_end == NULL) {
cinfo->marker_list = cinfo->master->marker_list_end = cur_marker;
} else {
jpeg_saved_marker_ptr prev = cinfo->marker_list;
while (prev->next != NULL)
prev = prev->next;
prev->next = cur_marker;
cinfo->master->marker_list_end->next = cur_marker;
cinfo->master->marker_list_end = cur_marker;
}
/* Reset pointer & calc remaining data length */
data = cur_marker->data;