From 0fc7313e545a3ff499c19ee6591bb87f0ad8b2a4 Mon Sep 17 00:00:00 2001 From: DRC Date: Tue, 14 May 2024 11:41:16 -0400 Subject: [PATCH] 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 --- jcomapi.c | 5 +++-- jdmarker.c | 12 +++++------- jpegint.h | 5 ++++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/jcomapi.c b/jcomapi.c index efbb8357..84f37e17 100644 --- a/jcomapi.c +++ b/jcomapi.c @@ -3,8 +3,8 @@ * * This file was part of the Independent JPEG Group's software: * Copyright (C) 1994-1997, Thomas G. Lane. - * It was modified by The libjpeg-turbo Project to include only code relevant - * to libjpeg-turbo. + * libjpeg-turbo Modifications: + * Copyright (C) 2024, D. R. Commander. * For conditions of distribution and use, see the accompanying README.ijg * file. * @@ -51,6 +51,7 @@ jpeg_abort(j_common_ptr cinfo) * A bit kludgy to do it here, but this is the most central place. */ ((j_decompress_ptr)cinfo)->marker_list = NULL; + ((j_decompress_ptr)cinfo)->master->marker_list_end = NULL; } else { cinfo->global_state = CSTATE_START; } diff --git a/jdmarker.c b/jdmarker.c index acd28ce6..ee6b6d41 100644 --- a/jdmarker.c +++ b/jdmarker.c @@ -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; diff --git a/jpegint.h b/jpegint.h index 65414201..7c28da3d 100644 --- a/jpegint.h +++ b/jpegint.h @@ -7,7 +7,7 @@ * Lossless JPEG Modifications: * Copyright (C) 1999, Ken Murchison. * libjpeg-turbo Modifications: - * Copyright (C) 2015-2017, 2019, 2021-2022, D. R. Commander. + * Copyright (C) 2015-2017, 2019, 2021-2022, 2024, D. R. Commander. * Copyright (C) 2015, Google, Inc. * Copyright (C) 2021, Alex Richardson. * For conditions of distribution and use, see the accompanying README.ijg @@ -249,6 +249,9 @@ struct jpeg_decomp_master { /* Last iMCU row that was successfully decoded */ JDIMENSION last_good_iMCU_row; + + /* Tail of list of saved markers */ + jpeg_saved_marker_ptr marker_list_end; }; /* Input control module */