From 570df030c0d30c2bf5336b2ab59f3902d8ab0b6d Mon Sep 17 00:00:00 2001 From: Jeffrey 'Alex' Clark Date: Mon, 12 Oct 2026 21:18:41 -0400 Subject: [PATCH] Validate TIFF tag values against libtiff's own field definition before TIFFSetField PyImaging_LibTiffEncoderNew chose the TIFFSetField() argument shape (scalar, tuple, or bytes) from the Python value alone, assuming its own ImagingLibTiffMergeFieldInfo() registration would determine how libtiff reads the varargs. It does not: TIFFMergeFieldInfo() never overrides libtiff's built-in definition for a tag it already knows, so libtiff reads the varargs according to its own field table regardless of what Pillow pushed. For a counted array tag such as DNGPrivateData (50740), a scalar integer from the source file was taken by libtiff as a byte count, and the data pointer was read from an argument slot Pillow never wrote -- copying that many bytes of process memory into the output tag. For a fixed-size array tag such as ActiveArea (50829), the integer was used directly as a source address, causing a segfault. Add ImagingLibTiffFindField() to look up libtiff's actual field definition (type, passcount, writecount) for a tag, and validate the Python value against it before calling TIFFSetField, raising ValueError instead of forwarding a mismatched value. The core tags that have their own dedicated branches (ColorMap, YCbCrSubsampling, ReferenceBlackWhite) are shape-checked too; ColorMap length is already validated in Python. ReferenceBlackWhite no longer needs to be blocklisted and is now written. TransferFunction remains unsupported by the libtiff encoder and stays blocklisted, so it is still dropped. Reported-by: carlini Fixes: GHSA-xvh5-g6gv-559v Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 5.5 --- Tests/test_file_libtiff.py | 95 ++++++++++++++++ src/PIL/TiffImagePlugin.py | 5 +- src/encode.c | 215 +++++++++++++++++++++++++++++++----- src/libImaging/TiffDecode.c | 23 ++++ src/libImaging/TiffDecode.h | 8 ++ 5 files changed, 314 insertions(+), 32 deletions(-) diff --git a/Tests/test_file_libtiff.py b/Tests/test_file_libtiff.py index ea56a1de34b..968bad5e2ae 100644 --- a/Tests/test_file_libtiff.py +++ b/Tests/test_file_libtiff.py @@ -415,6 +415,101 @@ def test_geometry_tags( with pytest.raises(OSError, match="encoder error -8"): im.save(out, tiffinfo=tiffinfo) + @pytest.mark.parametrize( + "tag, value", + ( + # DNGPrivateData is a BYTE array with a count to libtiff + (50740, 4096), + # ActiveArea is a LONG array of four values to libtiff + (50829, 64), + (50829, (0, 0)), + ), + ) + def test_libtiff_field_definition( + self, tag: int, value: Any, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + monkeypatch.setattr(TiffImagePlugin, "WRITE_LIBTIFF", True) + + ifd = TiffImagePlugin.ImageFileDirectory_v2() + ifd[tag] = value + ifd.tagtype[tag] = TiffTags.LONG + + out = tmp_path / "temp.tif" + im = Image.new("L", (1, 1)) + with pytest.raises(ValueError, match="libtiff field definition"): + im.save(out, tiffinfo=ifd) + + # Values that match the libtiff definition are written + ifd = TiffImagePlugin.ImageFileDirectory_v2() + ifd[50740] = b"private" + ifd.tagtype[50740] = TiffTags.BYTE + ifd[50829] = (0, 0, 1, 1) + ifd.tagtype[50829] = TiffTags.LONG + im.save(out, tiffinfo=ifd) + + with Image.open(out) as reloaded: + assert isinstance(reloaded, TiffImagePlugin.TiffImageFile) + assert reloaded.tag_v2[50740] == b"private" + assert reloaded.tag_v2[50829] == (0, 0, 1, 1) + + def test_transferfunction_not_saved( + self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path + ) -> None: + # TransferFunction is not supported by the libtiff encoder, so it is + # dropped rather than raising an error + monkeypatch.setattr(TiffImagePlugin, "WRITE_LIBTIFF", True) + + out = tmp_path / "temp.tif" + im = Image.new("L", (1, 1)) + im.save(out, tiffinfo={TiffImagePlugin.TRANSFERFUNCTION: tuple(range(256))}) + + with Image.open(out) as reloaded: + assert isinstance(reloaded, TiffImagePlugin.TiffImageFile) + assert TiffImagePlugin.TRANSFERFUNCTION not in reloaded.tag_v2 + + @pytest.mark.parametrize( + "tag, mode, value, invalid_value", + ( + (320, "L", tuple((i % 256) * 256 for i in range(768)), (0, 0, 0)), + (530, "YCbCr", (1, 1), (1,)), + (532, "YCbCr", (0, 255, 128, 255, 128, 255), (0, 255)), + ), + ) + def test_libtiff_core_field_definition( + self, + tag: int, + mode: str, + value: tuple[int, ...], + invalid_value: tuple[int, ...], + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + ) -> None: + monkeypatch.setattr(TiffImagePlugin, "WRITE_LIBTIFF", True) + + out = tmp_path / "temp.tif" + im = Image.new(mode, (1, 1)) + + ifd = TiffImagePlugin.ImageFileDirectory_v2() + ifd[tag] = invalid_value + # The ColorMap length is checked in Python first + match = "Requiring 768 items" if tag == 320 else "libtiff field definition" + with pytest.raises(ValueError, match=match): + im.save(out, tiffinfo=ifd) + + ifd = TiffImagePlugin.ImageFileDirectory_v2() + ifd[tag] = value + + im.save(out, tiffinfo=ifd) + + with Image.open(out) as reloaded: + assert isinstance(reloaded, TiffImagePlugin.TiffImageFile) + if tag == 532: + assert tuple(float(v) for v in reloaded.tag_v2[tag]) == pytest.approx( + value + ) + else: + assert tuple(reloaded.tag_v2[tag]) == value + def test_inknames_tag( self, monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: diff --git a/src/PIL/TiffImagePlugin.py b/src/PIL/TiffImagePlugin.py index 1383c81037b..6305b42e3f2 100644 --- a/src/PIL/TiffImagePlugin.py +++ b/src/PIL/TiffImagePlugin.py @@ -1981,13 +1981,10 @@ def _save(im: Image.Image, fp: IO[bytes], filename: str | bytes) -> None: # STRIPOFFSETS and STRIPBYTECOUNTS are added by the library # based on the data in the strip. # OSUBFILETYPE is deprecated. - # The other tags expect arrays with a certain length (fixed or depending on - # BITSPERSAMPLE, etc), passing arrays with a different length will result in - # segfaults. Block these tags until we add extra validation. + # TRANSFERFUNCTION is not supported by the libtiff encoder. # SUBIFD may also cause a segfault. blocklist += [ OSUBFILETYPE, - REFERENCEBLACKWHITE, STRIPBYTECOUNTS, STRIPOFFSETS, TRANSFERFUNCTION, diff --git a/src/encode.c b/src/encode.c index ab05fac2a88..28b6762d51e 100644 --- a/src/encode.c +++ b/src/encode.c @@ -677,6 +677,127 @@ PyImaging_ZipEncoderNew(PyObject *self, PyObject *args) { #include +/* C type that TIFFSetField reads from its arguments for a single value of + the given type: int, uint64_t, double or char * */ +static int +tiff_scalar_arg_type(TIFFDataType type) { + switch (type) { + case TIFF_BYTE: + case TIFF_SBYTE: + case TIFF_UNDEFINED: + case TIFF_SHORT: + case TIFF_SSHORT: + case TIFF_LONG: + case TIFF_SLONG: + case TIFF_IFD: + return 1; + case TIFF_LONG8: + case TIFF_SLONG8: + case TIFF_IFD8: + return 2; + case TIFF_FLOAT: + case TIFF_DOUBLE: + case TIFF_RATIONAL: + case TIFF_SRATIONAL: + return 3; + case TIFF_ASCII: + return 4; + default: + return 0; + } +} + +/* Size of the values that TIFFSetField copies from an array of the given + type. libtiff stores custom RATIONAL arrays as floats. */ +static int +tiff_array_item_size(TIFFDataType type) { + switch (type) { + case TIFF_BYTE: + case TIFF_SBYTE: + case TIFF_UNDEFINED: + case TIFF_ASCII: + return 1; + case TIFF_SHORT: + case TIFF_SSHORT: + return 2; + case TIFF_LONG: + case TIFF_SLONG: + case TIFF_IFD: + case TIFF_FLOAT: + case TIFF_RATIONAL: + case TIFF_SRATIONAL: + return 4; + case TIFF_DOUBLE: + case TIFF_LONG8: + case TIFF_SLONG8: + case TIFF_IFD8: + return 8; + default: + return 0; + } +} + +/* Check that a value which will be passed to TIFFSetField as a single value + of the given type, or as an array of len values, matches the definition + of the field that libtiff will use. */ +static int +tiff_needs_field_definition(int is_core_tag, int key) { + return !is_core_tag || key == TIFFTAG_TRANSFERFUNCTION || key == TIFFTAG_COLORMAP || + key == TIFFTAG_YCBCRSUBSAMPLING || key == TIFFTAG_REFERENCEBLACKWHITE; +} + +static int +tiff_special_field_matches( + int key, TIFFDataType type, int is_var_length, Py_ssize_t len, long bits_per_sample +) { + if (key == TIFFTAG_COLORMAP) { + // libtiff reads three arrays of 1 << BitsPerSample values + return is_var_length && type == TIFF_SHORT && bits_per_sample >= 1 && + bits_per_sample <= 16 && len == 3 * ((Py_ssize_t)1 << bits_per_sample); + } + if (key == TIFFTAG_YCBCRSUBSAMPLING) { + return is_var_length && type == TIFF_SHORT && len == 2; + } + return 0; +} + +static int +tiff_field_matches( + TIFFDataType field_type, + int passcount, + int writecount, + TIFFDataType type, + int is_var_length, + Py_ssize_t len +) { + if (field_type == TIFF_ASCII && !passcount) { + // TIFFSetField reads a null-terminated string + return !is_var_length && type == TIFF_ASCII; + } + if (!is_var_length) { + // TIFFSetField reads a single value + return !passcount && writecount == 1 && tiff_scalar_arg_type(type) && + tiff_scalar_arg_type(type) == tiff_scalar_arg_type(field_type); + } + // TIFFSetField reads a pointer to the values, preceded by the number of + // values if passcount is set. Otherwise, the number of values is fixed. + if (!tiff_array_item_size(type) || + tiff_array_item_size(type) != tiff_array_item_size(field_type)) { + return 0; + } + return passcount || (writecount > 1 && len == writecount); +} + +static int +set_tiff_array_field( + ImagingCodecState state, int key, int passcount, Py_ssize_t len, void *av +) { + if (passcount) { + return ImagingLibTiffSetField(state, (ttag_t)key, len, av); + } + return ImagingLibTiffSetField(state, (ttag_t)key, av); +} + PyObject * PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { ImagingEncoderObject *encoder; @@ -690,7 +811,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { PyObject *key, *value; Py_ssize_t pos = 0; int key_int, status, is_core_tag, is_var_length, num_core_tags, i; - TIFFDataType type = TIFF_NOTYPE; + int is_known_tag, passcount, writecount; + TIFFDataType type = TIFF_NOTYPE, field_type; // This list also exists in TiffTags.py const int core_tags[] = {256, 257, 258, 259, 262, 263, 266, 269, 274, 277, 278, 280, 281, 282, 283, 284, 286, 287, 296, 297, @@ -768,7 +890,11 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { status = 0; is_core_tag = 0; is_var_length = 0; + is_known_tag = 0; + passcount = 1; + writecount = 0; type = TIFF_NOTYPE; + field_type = TIFF_NOTYPE; for (i = 0; i < num_core_tags; i++) { if (core_tags[i] == key_int) { @@ -777,6 +903,15 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { } } + if (tiff_needs_field_definition(is_core_tag, key_int)) { + is_known_tag = ImagingLibTiffFindField( + &encoder->state, key_int, &field_type, &passcount, &writecount + ); + if (is_known_tag && type == TIFF_NOTYPE) { + type = field_type; + } + } + if (!is_core_tag) { PyObject *tag_type; if (PyDict_GetItemRef(types, key, &tag_type) < 0) { @@ -827,16 +962,38 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { } } - if (!is_core_tag) { - // Register field for non core tags. - if (type == TIFF_BYTE) { - is_var_length = 1; + if (type == TIFF_BYTE || type == TIFF_UNDEFINED) { + is_var_length = 1; + } + if (!is_core_tag && ImagingLibTiffMergeFieldInfo( + &encoder->state, type, key_int, is_var_length + )) { + Py_DECREF(item); + continue; + } + + if (is_known_tag) { + // Check that the value will be passed to TIFFSetField in the + // form that libtiff's definition of the tag expects. + Py_ssize_t len = 0; + if (PyTuple_Check(value)) { + len = PyTuple_Size(value); + } else if (PyBytes_Check(value)) { + len = PyBytes_Size(value); } - if (ImagingLibTiffMergeFieldInfo( - &encoder->state, type, key_int, is_var_length - )) { + if (!(tiff_special_field_matches( + key_int, type, is_var_length, len, bits_per_sample + ) || + tiff_field_matches( + field_type, passcount, writecount, type, is_var_length, len + ))) { + Py_DECREF(encoder); Py_DECREF(item); - continue; + PyErr_SetString( + PyExc_ValueError, + "Tag value does not match libtiff field definition" + ); + return NULL; } } @@ -848,9 +1005,10 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { PyErr_SetString(PyExc_ValueError, "Incorrect tag value type"); return NULL; } - status = ImagingLibTiffSetField( + status = set_tiff_array_field( &encoder->state, - (ttag_t)key_int, + key_int, + passcount, PyBytes_Size(value), PyBytes_AsString(value) ); @@ -891,8 +1049,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (UINT16)PyLong_AsLong(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -904,8 +1062,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (UINT32)PyLong_AsLong(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -917,8 +1075,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (INT8)PyLong_AsLong(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -930,8 +1088,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (INT16)PyLong_AsLong(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -943,8 +1101,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (INT32)PyLong_AsLong(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -956,8 +1114,8 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (FLOAT32)PyFloat_AsDouble(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } @@ -969,12 +1127,12 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = PyFloat_AsDouble(PyTuple_GetItem(value, i)); } - status = ImagingLibTiffSetField( - &encoder->state, (ttag_t)key_int, len, av + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av ); free(av); } - } else if (type == TIFF_RATIONAL) { + } else if (type == TIFF_RATIONAL || type == TIFF_SRATIONAL) { FLOAT32 *av; /* malloc check ok, calloc checks for overflow */ av = calloc(len, sizeof(FLOAT32)); @@ -982,8 +1140,9 @@ PyImaging_LibTiffEncoderNew(PyObject *self, PyObject *args) { for (i = 0; i < len; i++) { av[i] = (FLOAT32)PyFloat_AsDouble(PyTuple_GetItem(value, i)); } - status = - ImagingLibTiffSetField(&encoder->state, (ttag_t)key_int, av); + status = set_tiff_array_field( + &encoder->state, key_int, passcount, len, av + ); free(av); } } diff --git a/src/libImaging/TiffDecode.c b/src/libImaging/TiffDecode.c index dabdb97fa7b..137ea0052e4 100644 --- a/src/libImaging/TiffDecode.c +++ b/src/libImaging/TiffDecode.c @@ -768,6 +768,29 @@ ImagingLibTiffMergeFieldInfo( return TIFFMergeFieldInfo(clientstate->tiff, info, n); } +int +ImagingLibTiffFindField( + ImagingCodecState state, + int key, + TIFFDataType *field_type, + int *passcount, + int *writecount +) { + // Look up libtiff's own definition of a tag. TIFFMergeFieldInfo does not + // replace the definition of a tag that libtiff already knows, so this is + // what TIFFSetField will use for such tags. + TIFFSTATE *clientstate = (TIFFSTATE *)state->context; + const TIFFField *fip = TIFFFindField(clientstate->tiff, (ttag_t)key, TIFF_ANY); + + if (!fip) { + return 0; + } + *field_type = TIFFFieldDataType(fip); + *passcount = TIFFFieldPassCount(fip); + *writecount = TIFFFieldWriteCount(fip); + return 1; +} + int ImagingLibTiffSetField(ImagingCodecState state, ttag_t tag, ...) { // after tif_dir.c->TIFFSetField. diff --git a/src/libImaging/TiffDecode.h b/src/libImaging/TiffDecode.h index c0ea9066dca..2a8a1813b11 100644 --- a/src/libImaging/TiffDecode.h +++ b/src/libImaging/TiffDecode.h @@ -41,6 +41,14 @@ ImagingLibTiffMergeFieldInfo( ImagingCodecState state, TIFFDataType field_type, int key, int is_var_length ); extern int +ImagingLibTiffFindField( + ImagingCodecState state, + int key, + TIFFDataType *field_type, + int *passcount, + int *writecount +); +extern int ImagingLibTiffSetField(ImagingCodecState state, ttag_t tag, ...); #endif