From 4935e606c632e38af8fca1729f8688fa0d152b90 Mon Sep 17 00:00:00 2001 From: dsinclair Date: Thu, 15 Sep 2016 12:21:39 -0700 Subject: Use safe math when rendering line segments in AGG. It is possible for the calculations in outline_aa::render_line to overflow as the |p| variable is calculated. This Cl updates the routine to use checked math when calculating the value of |p|. BUG=chromium:647026 Review-Url: https://codereview.chromium.org/2347603002 --- .../agg23/0003-ubsan-render-line-error.patch | 73 ++++++++++++++++++++++ third_party/agg23/README.pdfium | 1 + third_party/agg23/agg_rasterizer_scanline_aa.cpp | 38 ++++++----- 3 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 third_party/agg23/0003-ubsan-render-line-error.patch (limited to 'third_party') diff --git a/third_party/agg23/0003-ubsan-render-line-error.patch b/third_party/agg23/0003-ubsan-render-line-error.patch new file mode 100644 index 0000000000..7bd3b70424 --- /dev/null +++ b/third_party/agg23/0003-ubsan-render-line-error.patch @@ -0,0 +1,73 @@ +diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.cpp b/third_party/agg23/agg_rasterizer_scanline_aa.cpp +index 46379f6..c6b3f01 100644 +--- a/third_party/agg23/agg_rasterizer_scanline_aa.cpp ++++ b/third_party/agg23/agg_rasterizer_scanline_aa.cpp +@@ -48,6 +48,7 @@ + //---------------------------------------------------------------------------- + #include + #include "agg_rasterizer_scanline_aa.h" ++#include "third_party/base/numerics/safe_math.h" + namespace agg + { + AGG_INLINE void cell_aa::set_cover(int c, int a) +@@ -237,7 +238,7 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) + int fy1 = y1 & poly_base_mask; + int fy2 = y2 & poly_base_mask; + int x_from, x_to; +- int p, rem, mod, lift, delta, first, incr; ++ int rem, mod, lift, delta, first, incr; + if(ey1 == ey2) { + render_hline(ey1, x1, fy1, x2, fy2); + return; +@@ -268,16 +269,22 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) + m_cur_cell.add_cover(delta, two_fx * delta); + return; + } +- p = (poly_base_size - fy1) * dx; ++ pdfium::base::CheckedNumeric safeP = poly_base_size - fy1; ++ safeP *= dx; ++ if (!safeP.IsValid()) ++ return; + first = poly_base_size; + if(dy < 0) { +- p = fy1 * dx; +- first = 0; +- incr = -1; +- dy = -dy; ++ safeP = fy1; ++ safeP *= dx; ++ if (!safeP.IsValid()) ++ return; ++ first = 0; ++ incr = -1; ++ dy = -dy; + } +- delta = p / dy; +- mod = p % dy; ++ delta = safeP.ValueOrDie() / dy; ++ mod = safeP.ValueOrDie() % dy; + if(mod < 0) { + delta--; + mod += dy; +@@ -287,12 +294,15 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) + ey1 += incr; + set_cur_cell(x_from >> poly_base_shift, ey1); + if(ey1 != ey2) { +- p = poly_base_size * dx; +- lift = p / dy; +- rem = p % dy; +- if(rem < 0) { +- lift--; +- rem += dy; ++ safeP = static_cast(poly_base_size); ++ safeP *= dx; ++ if (!safeP.IsValid()) ++ return; ++ lift = safeP.ValueOrDie() / dy; ++ rem = safeP.ValueOrDie() % dy; ++ if (rem < 0) { ++ lift--; ++ rem += dy; + } + mod -= dy; + while(ey1 != ey2) { diff --git a/third_party/agg23/README.pdfium b/third_party/agg23/README.pdfium index 8e055d2079..4b1ff49146 100644 --- a/third_party/agg23/README.pdfium +++ b/third_party/agg23/README.pdfium @@ -15,3 +15,4 @@ Possibly more? 0001-gcc-warning.patch: Fix a GCC warning about both enumeral and non-enumeral type in conditional. 0002-ubsan-error-fixes.path: Fix UBSan errors for overflows. +0003-ubsan-render-line-error.patch: Fix UBSan overflow error in render_line. diff --git a/third_party/agg23/agg_rasterizer_scanline_aa.cpp b/third_party/agg23/agg_rasterizer_scanline_aa.cpp index 46379f6dfd..c6b3f013a0 100644 --- a/third_party/agg23/agg_rasterizer_scanline_aa.cpp +++ b/third_party/agg23/agg_rasterizer_scanline_aa.cpp @@ -48,6 +48,7 @@ //---------------------------------------------------------------------------- #include #include "agg_rasterizer_scanline_aa.h" +#include "third_party/base/numerics/safe_math.h" namespace agg { AGG_INLINE void cell_aa::set_cover(int c, int a) @@ -237,7 +238,7 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) int fy1 = y1 & poly_base_mask; int fy2 = y2 & poly_base_mask; int x_from, x_to; - int p, rem, mod, lift, delta, first, incr; + int rem, mod, lift, delta, first, incr; if(ey1 == ey2) { render_hline(ey1, x1, fy1, x2, fy2); return; @@ -268,16 +269,22 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) m_cur_cell.add_cover(delta, two_fx * delta); return; } - p = (poly_base_size - fy1) * dx; + pdfium::base::CheckedNumeric safeP = poly_base_size - fy1; + safeP *= dx; + if (!safeP.IsValid()) + return; first = poly_base_size; if(dy < 0) { - p = fy1 * dx; - first = 0; - incr = -1; - dy = -dy; + safeP = fy1; + safeP *= dx; + if (!safeP.IsValid()) + return; + first = 0; + incr = -1; + dy = -dy; } - delta = p / dy; - mod = p % dy; + delta = safeP.ValueOrDie() / dy; + mod = safeP.ValueOrDie() % dy; if(mod < 0) { delta--; mod += dy; @@ -287,12 +294,15 @@ void outline_aa::render_line(int x1, int y1, int x2, int y2) ey1 += incr; set_cur_cell(x_from >> poly_base_shift, ey1); if(ey1 != ey2) { - p = poly_base_size * dx; - lift = p / dy; - rem = p % dy; - if(rem < 0) { - lift--; - rem += dy; + safeP = static_cast(poly_base_size); + safeP *= dx; + if (!safeP.IsValid()) + return; + lift = safeP.ValueOrDie() / dy; + rem = safeP.ValueOrDie() % dy; + if (rem < 0) { + lift--; + rem += dy; } mod -= dy; while(ey1 != ey2) { -- cgit v1.2.3