Fix unsafe buffer usage in CPDF_RenderShading Convert to array<> and span<> as needed. Change-Id: I85b008df9d67a4a5b1bd8f1ccf7f6043b8fd174b Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119850 Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com> Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/render/cpdf_rendershading.cpp b/core/fpdfapi/render/cpdf_rendershading.cpp index 79f4966..325cee1 100644 --- a/core/fpdfapi/render/cpdf_rendershading.cpp +++ b/core/fpdfapi/render/cpdf_rendershading.cpp
@@ -4,11 +4,6 @@ // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com -#if defined(UNSAFE_BUFFERS_BUILD) -// TODO(crbug.com/pdfium/2154): resolve buffer safety issues. -#pragma allow_unsafe_buffers -#endif - #include "core/fpdfapi/render/cpdf_rendershading.h" #include <math.h> @@ -31,6 +26,7 @@ #include "core/fpdfapi/render/cpdf_renderoptions.h" #include "core/fxcrt/check.h" #include "core/fxcrt/check_op.h" +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/fx_system.h" #include "core/fxcrt/numerics/clamped_math.h" @@ -139,28 +135,30 @@ CFX_Matrix matrix = mtObject2Bitmap.GetInverse(); for (int row = 0; row < height; row++) { - uint32_t* dib_buf = + auto dest_buf = fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row)) - .data(); - for (int column = 0; column < width; column++) { - CFX_PointF pos = matrix.Transform( - CFX_PointF(static_cast<float>(column), static_cast<float>(row))); + .first(width); + size_t column_counter = 0; + for (auto& pix : dest_buf) { + const float column = static_cast<float>(column_counter++); + const CFX_PointF pos = + matrix.Transform(CFX_PointF(column, static_cast<float>(row))); float scale = (((pos.x - start_x) * x_span) + ((pos.y - start_y) * y_span)) / axis_len_square; int index = static_cast<int32_t>(scale * (kShadingSteps - 1)); if (index < 0) { - if (!bStartExtend) + if (!bStartExtend) { continue; - + } index = 0; } else if (index >= kShadingSteps) { - if (!bEndExtend) + if (!bEndExtend) { continue; - + } index = kShadingSteps - 1; } - dib_buf[column] = shading_steps[index]; + pix = shading_steps[index]; } } } @@ -213,12 +211,14 @@ CFX_Matrix matrix = mtObject2Bitmap.GetInverse(); for (int row = 0; row < height; row++) { - uint32_t* dib_buf = + auto dest_buf = fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row)) - .data(); - for (int column = 0; column < width; column++) { - CFX_PointF pos = matrix.Transform( - CFX_PointF(static_cast<float>(column), static_cast<float>(row))); + .first(width); + size_t column_counter = 0; + for (auto& pix : dest_buf) { + const float column = static_cast<float>(column_counter++); + const CFX_PointF pos = + matrix.Transform(CFX_PointF(column, static_cast<float>(row))); float pos_dx = pos.x - start_x; float pos_dy = pos.y - start_y; float b = -2 * (pos_dx * dx + pos_dy * dy + start_r * dr); @@ -230,34 +230,36 @@ s = -c / b; } else { float b2_4ac = (b * b) - 4 * (a * c); - if (b2_4ac < 0) + if (b2_4ac < 0) { continue; - + } float root = sqrt(b2_4ac); float s1 = (-b - root) / (2 * a); float s2 = (-b + root) / (2 * a); if (a <= 0) std::swap(s1, s2); - if (bDecreasing) + if (bDecreasing) { s = (s1 >= 0 || bStartExtend) ? s1 : s2; - else + } else { s = (s2 <= 1.0f || bEndExtend) ? s2 : s1; - - if (start_r + s * dr < 0) + } + if (start_r + s * dr < 0) { continue; + } } - int index = static_cast<int32_t>(s * (kShadingSteps - 1)); if (index < 0) { - if (!bStartExtend) + if (!bStartExtend) { continue; + } index = 0; } else if (index >= kShadingSteps) { - if (!bEndExtend) + if (!bEndExtend) { continue; + } index = kShadingSteps - 1; } - dib_buf[column] = shading_steps[index]; + pix = shading_steps[index]; } } } @@ -295,9 +297,8 @@ CHECK_GE(total_results, pCS->ComponentCount()); std::vector<float> result_array(total_results); for (int row = 0; row < height; ++row) { - uint32_t* dib_buf = - fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row)) - .data(); + auto dib_buf = + fxcrt::reinterpret_span<uint32_t>(pBitmap->GetWritableScanline(row)); for (int column = 0; column < width; column++) { CFX_PointF pos = matrix.Transform( CFX_PointF(static_cast<float>(column), static_cast<float>(row))); @@ -343,7 +344,7 @@ void DrawGouraud(const RetainPtr<CFX_DIBitmap>& pBitmap, int alpha, - CPDF_MeshVertex triangle[3]) { + pdfium::span<CPDF_MeshVertex, 3> triangle) { float min_y = triangle[0].position.y; float max_y = triangle[0].position.y; for (int i = 1; i < 3; i++) { @@ -360,10 +361,10 @@ for (int y = min_yi; y <= max_yi; y++) { int nIntersects = 0; - float inter_x[3]; - float r[3]; - float g[3]; - float b[3]; + std::array<float, 3> inter_x; + std::array<float, 3> r; + std::array<float, 3> g; + std::array<float, 3> b; for (int i = 0; i < 3; i++) { const CPDF_MeshVertex& vertex1 = triangle[i]; const CPDF_MeshVertex& vertex2 = triangle[(i + 1) % 3]; @@ -416,13 +417,13 @@ pBitmap->GetWritableScanline(y).subspan(start_x * 4); for (int x = start_x; x < end_x; x++) { - uint8_t* dib_buf = dib_span.data(); r_result += r_unit; g_result += g_unit; b_result += b_unit; - FXARGB_SetDIB(dib_buf, ArgbEncode(alpha, static_cast<int>(r_result * 255), - static_cast<int>(g_result * 255), - static_cast<int>(b_result * 255))); + UNSAFE_TODO(FXARGB_SetDIB( + dib_span.data(), ArgbEncode(alpha, static_cast<int>(r_result * 255), + static_cast<int>(g_result * 255), + static_cast<int>(b_result * 255)))); dib_span = dib_span.subspan(4); } } @@ -442,7 +443,7 @@ if (!stream.Load()) return; - CPDF_MeshVertex triangle[3]; + std::array<CPDF_MeshVertex, 3> triangle; while (!stream.IsEOF()) { CPDF_MeshVertex vertex; uint32_t flag; @@ -485,7 +486,7 @@ if (!stream.Load()) return; - std::vector<CPDF_MeshVertex> vertices[2]; + std::array<std::vector<CPDF_MeshVertex>, 2> vertices; vertices[0] = stream.ReadVertexRow(mtObject2Bitmap, row_verts); if (vertices[0].empty()) return; @@ -546,7 +547,7 @@ return result; } - void GetPoints(float p[4]) const { + void GetPoints(pdfium::span<float, 4> p) const { p[0] = d; p[1] = c / 3 + p[0]; p[2] = b / 3 - p[0] + 2 * p[1]; @@ -601,8 +602,8 @@ void GetPoints(pdfium::span<CFX_Path::Point> path_points) const { constexpr size_t kPointsCount = 4; - float points_x[kPointsCount]; - float points_y[kPointsCount]; + std::array<float, kPointsCount> points_x; + std::array<float, kPointsCount> points_y; x.GetPoints(points_x); y.GetPoints(points_y); for (size_t i = 0; i < kPointsCount; ++i) @@ -611,8 +612,8 @@ void GetPointsReverse(pdfium::span<CFX_Path::Point> path_points) const { constexpr size_t kPointsCount = 4; - float points_x[kPointsCount]; - float points_y[kPointsCount]; + std::array<float, kPointsCount> points_x; + std::array<float, kPointsCount> points_y; x.GetPoints(points_x); y.GetPoints(points_y); for (size_t i = 0; i < kPointsCount; ++i) { @@ -657,7 +658,11 @@ CoonColor() = default; // Returns true if successful, false if overflow detected. - bool BiInterpol(CoonColor colors[4], int x, int y, int x_scale, int y_scale) { + bool BiInterpol(pdfium::span<CoonColor, 4> colors, + int x, + int y, + int x_scale, + int y_scale) { bool overflow = false; for (int i = 0; i < 3; i++) { comp[i] = BiInterpolImpl(colors[0].comp[i], colors[1].comp[i], @@ -672,7 +677,7 @@ abs(comp[2] - o.comp[2])}); } - int comp[3] = {}; + std::array<int, 3> comp = {}; }; struct PatchDrawer { @@ -785,7 +790,7 @@ UnownedPtr<CFX_RenderDevice> pDevice; bool bNoPathSmooth; int alpha; - CoonColor patch_colors[4]; + std::array<CoonColor, 4> patch_colors; }; void DrawCoonPatchMeshes( @@ -820,7 +825,7 @@ : CFX_Path::Point::Type::kBezier); } - CFX_PointF coords[16]; + std::array<CFX_PointF, 16> coords; int point_count = type == kTensorProductPatchMeshShading ? 16 : 12; while (!stream.IsEOF()) { if (!stream.CanReadFlag()) @@ -832,14 +837,15 @@ if (flag) { iStartPoint = 4; iStartColor = 2; - CFX_PointF tempCoords[4]; + std::array<CFX_PointF, 4> tempCoords; for (i = 0; i < 4; i++) { tempCoords[i] = coords[(flag * 3 + i) % 12]; } fxcrt::spancpy(pdfium::make_span(coords), pdfium::make_span(tempCoords)); - CoonColor tempColors[2] = { - tempColors[0] = patch.patch_colors[flag], - tempColors[1] = patch.patch_colors[(flag + 1) % 4]}; + std::array<CoonColor, 2> tempColors = {{ + patch.patch_colors[flag], + patch.patch_colors[(flag + 1) % 4], + }}; fxcrt::spancpy(pdfium::make_span(patch.patch_colors), pdfium::make_span(tempColors)); }