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);
