Skip to content

Commit

Permalink
Don't allow opaque source/dest mgrs to be swapped
Browse files Browse the repository at this point in the history
Calling jpeg_stdio_dest() followed by jpeg_mem_dest(), or jpeg_mem_src()
followed by jpeg_stdio_src(), is dangerous, because the existing opaque
structure would not be big enough to accommodate the new source/dest
manager.  This issue was non-obvious to libjpeg-turbo consumers, since
it was only documented in code comments.  Furthermore, the issue could
also occur if the source/dest manager was allocated by the calling
program, but it was not allocated with enough space to accommodate the
opaque stdio or memory source/dest manager structs.  The safest thing to
do is to throw an error if one of these functions is called when there
is already a source/dest manager assigned to the object and it was
allocated elsewhere.

Closes libjpeg-turbo#78, libjpeg-turbo#79
  • Loading branch information
dcommander committed May 11, 2016
1 parent f06cc12 commit 68cf83d
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 9 deletions.
8 changes: 8 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ the TurboJPEG API.)
5. Fixed an issue in the ARM 32-bit SIMD-accelerated Huffman encoder that
prevented the code from assembling properly with clang.

6. The `jpeg_stdio_src()`, `jpeg_mem_src()`, `jpeg_stdio_dest()`, and
`jpeg_mem_dest()` functions in the libjpeg API will now throw an error if a
source/destination manager has already been assigned to the compress or
decompress object by a different function or by the calling program. This
prevents these functions from attempting to reuse a source/destination manager
structure that was allocated elsewhere, because there is no way to ensure that
it would be big enough to accommodate the new source/destination manager.


1.4.90 (1.5 beta1)
==================
Expand Down
7 changes: 6 additions & 1 deletion jdatadst-tj.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* Modified 2009-2012 by Guido Vollbeding.
* libjpeg-turbo Modifications:
* Copyright (C) 2011, 2014 D. R. Commander.
* Copyright (C) 2011, 2014, 2016, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand Down Expand Up @@ -167,6 +167,11 @@ jpeg_mem_dest_tj (j_compress_ptr cinfo,
dest = (my_mem_dest_ptr) cinfo->dest;
dest->newbuffer = NULL;
dest->buffer = NULL;
} else if (cinfo->dest->init_destination != init_mem_destination) {
/* It is unsafe to reuse the existing destination manager unless it was
* created by this function.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

dest = (my_mem_dest_ptr) cinfo->dest;
Expand Down
18 changes: 14 additions & 4 deletions jdatadst.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* Modified 2009-2012 by Guido Vollbeding.
* libjpeg-turbo Modifications:
* Copyright (C) 2013, D. R. Commander.
* Copyright (C) 2013, 2016, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand Down Expand Up @@ -210,14 +210,19 @@ jpeg_stdio_dest (j_compress_ptr cinfo, FILE *outfile)

/* The destination object is made permanent so that multiple JPEG images
* can be written to the same file without re-executing jpeg_stdio_dest.
* This makes it dangerous to use this manager and a different destination
* manager serially with the same JPEG object, because their private object
* sizes may be different. Caveat programmer.
*/
if (cinfo->dest == NULL) { /* first time for this JPEG object? */
cinfo->dest = (struct jpeg_destination_mgr *)
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
sizeof(my_destination_mgr));
} else if (cinfo->dest->init_destination != init_destination) {
/* It is unsafe to reuse the existing destination manager unless it was
* created by this function. Otherwise, there is no guarantee that the
* opaque structure is the right size. Note that we could just create a
* new structure, but the old structure would not be freed until
* jpeg_destroy_compress() was called.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

dest = (my_dest_ptr) cinfo->dest;
Expand Down Expand Up @@ -259,6 +264,11 @@ jpeg_mem_dest (j_compress_ptr cinfo,
cinfo->dest = (struct jpeg_destination_mgr *)
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
sizeof(my_mem_destination_mgr));
} else if (cinfo->dest->init_destination != init_mem_destination) {
/* It is unsafe to reuse the existing destination manager unless it was
* created by this function.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

dest = (my_mem_dest_ptr) cinfo->dest;
Expand Down
7 changes: 6 additions & 1 deletion jdatasrc-tj.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* Modified 2009-2011 by Guido Vollbeding.
* libjpeg-turbo Modifications:
* Copyright (C) 2011, D. R. Commander.
* Copyright (C) 2011, 2016, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand Down Expand Up @@ -173,6 +173,11 @@ jpeg_mem_src_tj (j_decompress_ptr cinfo,
cinfo->src = (struct jpeg_source_mgr *)
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
sizeof(struct jpeg_source_mgr));
} else if (cinfo->src->init_source != init_mem_source) {
/* It is unsafe to reuse the existing source manager unless it was created
* by this function.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

src = cinfo->src;
Expand Down
17 changes: 14 additions & 3 deletions jdatasrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* Copyright (C) 1994-1996, Thomas G. Lane.
* Modified 2009-2011 by Guido Vollbeding.
* libjpeg-turbo Modifications:
* Copyright (C) 2013, D. R. Commander.
* Copyright (C) 2013, 2016, D. R. Commander.
* For conditions of distribution and use, see the accompanying README.ijg
* file.
*
Expand Down Expand Up @@ -222,8 +222,6 @@ jpeg_stdio_src (j_decompress_ptr cinfo, FILE *infile)
* of JPEG images can be read from the same file by calling jpeg_stdio_src
* only before the first one. (If we discarded the buffer at the end of
* one image, we'd likely lose the start of the next one.)
* This makes it unsafe to use this manager and a different source
* manager serially with the same JPEG object. Caveat programmer.
*/
if (cinfo->src == NULL) { /* first time for this JPEG object? */
cinfo->src = (struct jpeg_source_mgr *)
Expand All @@ -233,6 +231,14 @@ jpeg_stdio_src (j_decompress_ptr cinfo, FILE *infile)
src->buffer = (JOCTET *)
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
INPUT_BUF_SIZE * sizeof(JOCTET));
} else if (cinfo->src->init_source != init_source) {
/* It is unsafe to reuse the existing source manager unless it was created
* by this function. Otherwise, there is no guarantee that the opaque
* structure is the right size. Note that we could just create a new
* structure, but the old structure would not be freed until
* jpeg_destroy_decompress() was called.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

src = (my_src_ptr) cinfo->src;
Expand Down Expand Up @@ -270,6 +276,11 @@ jpeg_mem_src (j_decompress_ptr cinfo,
cinfo->src = (struct jpeg_source_mgr *)
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
sizeof(struct jpeg_source_mgr));
} else if (cinfo->src->init_source != init_mem_source) {
/* It is unsafe to reuse the existing source manager unless it was created
* by this function.
*/
ERREXIT(cinfo, JERR_BUFFER_SIZE);
}

src = cinfo->src;
Expand Down

0 comments on commit 68cf83d

Please sign in to comment.