Skip to content

Commit

Permalink
tj3*YUV8(): Fix int overflow w/ huge row alignment
Browse files Browse the repository at this point in the history
If the align parameter was set to an unreasonably large value, such as
0x2000000, strides[0] * ph0 and strides[1] * ph1 could have overflowed
the int datatype and wrapped around when computing (src|dst)Planes[1]
and (src|dst)Planes[2] (respectively.)  This would have caused
(src|dst)Planes[1] and (src|dst)Planes[2] to point to lower addresses in
the YUV buffer than expected, so the worst case would have been a
visually incorrect output image, not a buffer overrun or other
exploitable issue.
  • Loading branch information
dcommander committed Nov 7, 2023
1 parent 0e2d289 commit 78eaf0d
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
13 changes: 13 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
3.0.2
=====

### Significant changes relative to 3.0.1:

1. Fixed a signed integer overflow in the `tj3CompressFromYUV8()`,
`tj3DecodeYUV8()`, `tj3DecompressToYUV8()`, and `tj3EncodeYUV8()` functions,
detected by the Clang and GCC undefined behavior sanitizers, that could be
triggered by setting the `align` parameter to an unreasonably large value.
This issue did not pose a security threat, but removing the warning made it
easier to detect actual security issues, should they arise in the future.


3.0.1
=====

Expand Down
20 changes: 20 additions & 0 deletions turbojpeg.c
Original file line number Diff line number Diff line change
Expand Up @@ -1407,6 +1407,11 @@ DLLEXPORT int tj3EncodeYUV8(tjhandle handle, const unsigned char *srcBuf,
int ph1 = tj3YUVPlaneHeight(1, height, this->subsamp);

strides[1] = strides[2] = PAD(pw1, align);
if ((unsigned long long)strides[0] * (unsigned long long)ph0 >
(unsigned long long)INT_MAX ||
(unsigned long long)strides[1] * (unsigned long long)ph1 >
(unsigned long long)INT_MAX)
THROW("Image or row alignment is too large");
dstPlanes[1] = dstPlanes[0] + strides[0] * ph0;
dstPlanes[2] = dstPlanes[1] + strides[1] * ph1;
}
Expand Down Expand Up @@ -1661,6 +1666,11 @@ DLLEXPORT int tj3CompressFromYUV8(tjhandle handle,
int ph1 = tjPlaneHeight(1, height, this->subsamp);

strides[1] = strides[2] = PAD(pw1, align);
if ((unsigned long long)strides[0] * (unsigned long long)ph0 >
(unsigned long long)INT_MAX ||
(unsigned long long)strides[1] * (unsigned long long)ph1 >
(unsigned long long)INT_MAX)
THROW("Image or row alignment is too large");
srcPlanes[1] = srcPlanes[0] + strides[0] * ph0;
srcPlanes[2] = srcPlanes[1] + strides[1] * ph1;
}
Expand Down Expand Up @@ -2242,6 +2252,11 @@ DLLEXPORT int tj3DecodeYUV8(tjhandle handle, const unsigned char *srcBuf,
int ph1 = tj3YUVPlaneHeight(1, height, this->subsamp);

strides[1] = strides[2] = PAD(pw1, align);
if ((unsigned long long)strides[0] * (unsigned long long)ph0 >
(unsigned long long)INT_MAX ||
(unsigned long long)strides[1] * (unsigned long long)ph1 >
(unsigned long long)INT_MAX)
THROW("Image or row alignment is too large");
srcPlanes[1] = srcPlanes[0] + strides[0] * ph0;
srcPlanes[2] = srcPlanes[1] + strides[1] * ph1;
}
Expand Down Expand Up @@ -2531,6 +2546,11 @@ DLLEXPORT int tj3DecompressToYUV8(tjhandle handle,
int ph1 = tj3YUVPlaneHeight(1, height, this->subsamp);

strides[1] = strides[2] = PAD(pw1, align);
if ((unsigned long long)strides[0] * (unsigned long long)ph0 >
(unsigned long long)INT_MAX ||
(unsigned long long)strides[1] * (unsigned long long)ph1 >
(unsigned long long)INT_MAX)
THROW("Image or row alignment is too large");
dstPlanes[1] = dstPlanes[0] + strides[0] * ph0;
dstPlanes[2] = dstPlanes[1] + strides[1] * ph1;
}
Expand Down

0 comments on commit 78eaf0d

Please sign in to comment.