Prevent another integer overflow in AGG. Avoid another situation in AGG's sweep_scanline() where signed integer math can result in undefined behavior. Bug: chromium:997021 Change-Id: Ica952174f34f0907598a1778cde0790de25768dd Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/59891 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/testing/resources/javascript/xfa_specific/bug_997021.in b/testing/resources/javascript/xfa_specific/bug_997021.in new file mode 100644 index 0000000..53ae098 --- /dev/null +++ b/testing/resources/javascript/xfa_specific/bug_997021.in
@@ -0,0 +1,30 @@ +{{header}} +{{include ../../xfa_catalog_1_0.fragment}} +{{include ../../xfa_object_2_0.fragment}} +{{include ../../xfa_preamble_3_0.fragment}} +{{include ../../xfa_config_4_0.fragment}} +{{object 5 0}} << + {{streamlen}} +>> +stream +<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/"> + <subform name="subform1"> + <field name="Field0"> + <ui> + <numericEdit> + <comb numberOfCells="64515" /> + </numericEdit> + </ui> + </field> + </subform> + <subform name="subform2" /> +</template> +endstream +endobj +{{include ../../xfa_locale_6_0.fragment}} +{{include ../../xfa_postamble_7_0.fragment}} +{{include ../../xfa_pages_8_0.fragment}} +{{xref}} +{{trailer}} +{{startxref}} +%%EOF
diff --git a/testing/resources/javascript/xfa_specific/bug_997021_expected.txt b/testing/resources/javascript/xfa_specific/bug_997021_expected.txt new file mode 100644 index 0000000..e69de29 --- /dev/null +++ b/testing/resources/javascript/xfa_specific/bug_997021_expected.txt
diff --git a/third_party/agg23/0006-ubsan-sweep-scanline-error.patch b/third_party/agg23/0006-ubsan-sweep-scanline-error.patch new file mode 100644 index 0000000..ec3707d --- /dev/null +++ b/third_party/agg23/0006-ubsan-sweep-scanline-error.patch
@@ -0,0 +1,70 @@ +diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.cpp b/third_party/agg23/agg_rasterizer_scanline_aa.cpp +index 1fe9a0c32..9254d830d 100644 +--- a/third_party/agg23/agg_rasterizer_scanline_aa.cpp ++++ b/third_party/agg23/agg_rasterizer_scanline_aa.cpp +@@ -502,4 +502,16 @@ int rasterizer_scanline_aa::calculate_area(int cover, int shift) + result <<= shift; + return result; + } ++// static ++bool rasterizer_scanline_aa::safe_add(int* op1, int op2) ++{ ++ pdfium::base::CheckedNumeric<int> safeOp1 = *op1; ++ safeOp1 += op2; ++ if(!safeOp1.IsValid()) { ++ return false; ++ } ++ ++ *op1 = safeOp1.ValueOrDie(); ++ return true; ++} + } +diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.h b/third_party/agg23/agg_rasterizer_scanline_aa.h +index 281933710..eade78333 100644 +--- a/third_party/agg23/agg_rasterizer_scanline_aa.h ++++ b/third_party/agg23/agg_rasterizer_scanline_aa.h +@@ -338,14 +338,33 @@ public: + const cell_aa* cur_cell = *cells; + int x = cur_cell->x; + int area = cur_cell->area; +- cover += cur_cell->cover; ++ bool seen_area_overflow = false; ++ bool seen_cover_overflow = false; ++ if(!safe_add(&cover, cur_cell->cover)) { ++ break; ++ } + while(--num_cells) { + cur_cell = *++cells; + if(cur_cell->x != x) { + break; + } +- area += cur_cell->area; +- cover += cur_cell->cover; ++ if(seen_area_overflow) { ++ continue; ++ } ++ if(!safe_add(&area, cur_cell->area)) { ++ seen_area_overflow = true; ++ continue; ++ } ++ if(!safe_add(&cover, cur_cell->cover)) { ++ seen_cover_overflow = true; ++ break; ++ } ++ } ++ if(seen_area_overflow) { ++ continue; ++ } ++ if(seen_cover_overflow) { ++ break; + } + if(area) { + unsigned alpha = calculate_alpha(calculate_area(cover, poly_base_shift + 1) - area, no_smooth); +@@ -459,6 +478,7 @@ private: + } + private: + static int calculate_area(int cover, int shift); ++ static bool safe_add(int* op1, int op2); + + outline_aa m_outline; + filling_rule_e m_filling_rule;
diff --git a/third_party/agg23/README.pdfium b/third_party/agg23/README.pdfium index c6212e2..12de429 100644 --- a/third_party/agg23/README.pdfium +++ b/third_party/agg23/README.pdfium
@@ -20,3 +20,5 @@ error in sweep_scanline. 0005-assignment-return-values.patch: Fix assignment operator return values in agg_array.h. +0006-ubsan-sweep-scanline-error.patch: Fix UBSan integer overflow error in +sweep_scanline.
diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.cpp b/third_party/agg23/agg_rasterizer_scanline_aa.cpp index 1fe9a0c..9254d83 100644 --- a/third_party/agg23/agg_rasterizer_scanline_aa.cpp +++ b/third_party/agg23/agg_rasterizer_scanline_aa.cpp
@@ -502,4 +502,16 @@ result <<= shift; return result; } +// static +bool rasterizer_scanline_aa::safe_add(int* op1, int op2) +{ + pdfium::base::CheckedNumeric<int> safeOp1 = *op1; + safeOp1 += op2; + if(!safeOp1.IsValid()) { + return false; + } + + *op1 = safeOp1.ValueOrDie(); + return true; +} }
diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.h b/third_party/agg23/agg_rasterizer_scanline_aa.h index 2819337..eade783 100644 --- a/third_party/agg23/agg_rasterizer_scanline_aa.h +++ b/third_party/agg23/agg_rasterizer_scanline_aa.h
@@ -338,14 +338,33 @@ const cell_aa* cur_cell = *cells; int x = cur_cell->x; int area = cur_cell->area; - cover += cur_cell->cover; + bool seen_area_overflow = false; + bool seen_cover_overflow = false; + if(!safe_add(&cover, cur_cell->cover)) { + break; + } while(--num_cells) { cur_cell = *++cells; if(cur_cell->x != x) { break; } - area += cur_cell->area; - cover += cur_cell->cover; + if(seen_area_overflow) { + continue; + } + if(!safe_add(&area, cur_cell->area)) { + seen_area_overflow = true; + continue; + } + if(!safe_add(&cover, cur_cell->cover)) { + seen_cover_overflow = true; + break; + } + } + if(seen_area_overflow) { + continue; + } + if(seen_cover_overflow) { + break; } if(area) { unsigned alpha = calculate_alpha(calculate_area(cover, poly_base_shift + 1) - area, no_smooth); @@ -459,6 +478,7 @@ } private: static int calculate_area(int cover, int shift); + static bool safe_add(int* op1, int op2); outline_aa m_outline; filling_rule_e m_filling_rule;