Fix integer overflow after recent change https://pdfium-review.googlesource.com/c/pdfium/+/134090 changed this code to used signed integers. But signed integer overflow isn't defined. Switch to int64_t. THe products are int32_t * uint16_t, and we add two of those and one int32_t. This easily fits into an int64_t. The first loop already naturally filters out very big and small x and y values. The second does too as long as we let CJBig2_Image::Compose*() take int64_ts, so switch to that. No intended behavior change, except for fuzzer (and fuzzer-like) inputs. Bug: 432691328 Change-Id: I22ee3bb5306409eb9a965f254d534a9dca9caa39 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/134470 Commit-Queue: Nico Weber <thakis@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Nico Weber <thakis@google.com>
diff --git a/core/fxcodec/jbig2/JBig2_HtrdProc.cpp b/core/fxcodec/jbig2/JBig2_HtrdProc.cpp index 9f764ac..6f9e796 100644 --- a/core/fxcodec/jbig2/JBig2_HtrdProc.cpp +++ b/core/fxcodec/jbig2/JBig2_HtrdProc.cpp
@@ -28,10 +28,12 @@ for (uint32_t ng = 0; ng < HGW; ++ng) { // The `>> 8` is an arithmetic shift per spec. Cast mg, ng to int, // else implicit conversions would evaluate it as unsigned shift. - int32_t mg_int = static_cast<int32_t>(mg); - int32_t ng_int = static_cast<int32_t>(ng); - int32_t x = (HGX + mg_int * HRY + ng_int * HRX) >> 8; - int32_t y = (HGY + mg_int * HRX - ng_int * HRY) >> 8; + // HGX / HGY are 32 bit, HRX / HRY 16, mg / ng 32. + // The result after >> 8 fits in about 42 bit; int64_t suffices. + auto mg_int = static_cast<int64_t>(mg); + auto ng_int = static_cast<int64_t>(ng); + int64_t x = (HGX + mg_int * HRY + ng_int * HRX) >> 8; + int64_t y = (HGY + mg_int * HRX - ng_int * HRY) >> 8; if ((x + HPW <= 0) | (x >= static_cast<int32_t>(HBW)) | (y + HPH <= 0) | (y >= static_cast<int32_t>(HBH))) { @@ -147,11 +149,13 @@ uint32_t pat_index = std::min(gsval, HNUMPATS - 1); // The `>> 8` is an arithmetic shift per spec. Cast mg, ng to int, // else implicit conversions would evaluate it as unsigned shift. - int32_t mg_int = static_cast<int32_t>(mg); - int32_t ng_int = static_cast<int32_t>(ng); - int32_t out_x = (HGX + mg_int * HRY + ng_int * HRX) >> 8; - int32_t out_y = (HGY + mg_int * HRX - ng_int * HRY) >> 8; - (*HPATS)[pat_index]->ComposeTo(HTREG.get(), out_x, out_y, HCOMBOP); + // HGX / HGY are 32 bit, HRX / HRY 16, mg / ng 32. + // The result after >> 8 fits in about 42 bit; int64_t suffices. + auto mg_int = static_cast<int64_t>(mg); + auto ng_int = static_cast<int64_t>(ng); + int64_t x = (HGX + mg_int * HRY + ng_int * HRX) >> 8; + int64_t y = (HGY + mg_int * HRX - ng_int * HRY) >> 8; + (*HPATS)[pat_index]->ComposeTo(HTREG.get(), x, y, HCOMBOP); } } return HTREG;
diff --git a/core/fxcodec/jbig2/JBig2_Image.cpp b/core/fxcodec/jbig2/JBig2_Image.cpp index 9883f9d..d1c2e4a 100644 --- a/core/fxcodec/jbig2/JBig2_Image.cpp +++ b/core/fxcodec/jbig2/JBig2_Image.cpp
@@ -176,30 +176,30 @@ } bool CJBig2_Image::ComposeTo(CJBig2_Image* pDst, - int32_t x, - int32_t y, + int64_t x, + int64_t y, JBig2ComposeOp op) { return data_ && ComposeToInternal(pDst, x, y, op, FX_RECT(0, 0, width_, height_)); } bool CJBig2_Image::ComposeToWithRect(CJBig2_Image* pDst, - int32_t x, - int32_t y, + int64_t x, + int64_t y, const FX_RECT& rtSrc, JBig2ComposeOp op) { return data_ && ComposeToInternal(pDst, x, y, op, rtSrc); } -bool CJBig2_Image::ComposeFrom(int32_t x, - int32_t y, +bool CJBig2_Image::ComposeFrom(int64_t x, + int64_t y, CJBig2_Image* pSrc, JBig2ComposeOp op) { return data_ && pSrc->ComposeTo(this, x, y, op); } -bool CJBig2_Image::ComposeFromWithRect(int32_t x, - int32_t y, +bool CJBig2_Image::ComposeFromWithRect(int64_t x, + int64_t y, CJBig2_Image* pSrc, const FX_RECT& rtSrc, JBig2ComposeOp op) { @@ -294,16 +294,18 @@ } bool CJBig2_Image::ComposeToInternal(CJBig2_Image* pDst, - int32_t x, - int32_t y, + int64_t x_in, + int64_t y_in, JBig2ComposeOp op, const FX_RECT& rtSrc) { DCHECK(data_); // TODO(weili): Check whether the range check is correct. Should x>=1048576? - if (x < -1048576 || x > 1048576 || y < -1048576 || y > 1048576) { + if (x_in < -1048576 || x_in > 1048576 || y_in < -1048576 || y_in > 1048576) { return false; } + int32_t x = static_cast<int32_t>(x_in); + int32_t y = static_cast<int32_t>(y_in); int32_t sw = rtSrc.Width(); int32_t sh = rtSrc.Height();
diff --git a/core/fxcodec/jbig2/JBig2_Image.h b/core/fxcodec/jbig2/JBig2_Image.h index 55d7e3e..7e72e9e 100644 --- a/core/fxcodec/jbig2/JBig2_Image.h +++ b/core/fxcodec/jbig2/JBig2_Image.h
@@ -62,9 +62,9 @@ void CopyLine(int32_t hTo, int32_t hFrom); void Fill(bool v); - bool ComposeFrom(int32_t x, int32_t y, CJBig2_Image* pSrc, JBig2ComposeOp op); - bool ComposeFromWithRect(int32_t x, - int32_t y, + bool ComposeFrom(int64_t x, int64_t y, CJBig2_Image* pSrc, JBig2ComposeOp op); + bool ComposeFromWithRect(int64_t x, + int64_t y, CJBig2_Image* pSrc, const FX_RECT& rtSrc, JBig2ComposeOp op); @@ -75,10 +75,10 @@ int32_t h); void Expand(int32_t h, bool v); - bool ComposeTo(CJBig2_Image* pDst, int32_t x, int32_t y, JBig2ComposeOp op); + bool ComposeTo(CJBig2_Image* pDst, int64_t x, int64_t y, JBig2ComposeOp op); bool ComposeToWithRect(CJBig2_Image* pDst, - int32_t x, - int32_t y, + int64_t x, + int64_t y, const FX_RECT& rtSrc, JBig2ComposeOp op); @@ -94,8 +94,8 @@ int32_t h, CJBig2_Image* pImage); bool ComposeToInternal(CJBig2_Image* pDst, - int32_t x, - int32_t y, + int64_t x_in, + int64_t y_in, JBig2ComposeOp op, const FX_RECT& rtSrc);