diff options
author | John Stiles <johnstiles@google.com> | 2023-04-12 14:56:54 -0400 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2023-05-02 08:59:24 +0000 |
commit | 5c93fd0f90d784bb2be6eccbc9452dbf19c70cd2 (patch) | |
tree | 87ca05021f3af7551d8f7106bfd22fbebb793f54 | |
parent | 3441a3e88b0be3c7eab697e564cb72445390d65d (diff) | |
download | qtwebengine-chromium-5c93fd0f90d784bb2be6eccbc9452dbf19c70cd2.tar.gz |
[Backport] CVE-2023-2136: Integer overflow in Skia (1/2)
Cherry-pick of patch originally reviewed on
https://skia-review.googlesource.com/c/skia/+/673576:
Enforce program stack limits on function parameters.
M108 merge issues:
resources/sksl/BUILD.bazel:
File doesn't exist in M108, tests are added manually to gn/sksl_tests.gni.
gn/sksl_tests.gni:
Conflicting rts entries
src/sksl/ir/SkSLFunctionDefinition.cpp:
- Conflicting includes
- visitStatement():
Conflicting declarations of const Variable* var (const Variable& var
on 108)
Previously, a function's parameter list did not count against its
stack size limit.
Bug: chromium:1432603
No-Try: true
No-Presubmit: true
No-Tree-Checks: true
Change-Id: If49dce98f3155f3144a766c26b5a3a39401ce1b2
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/670236
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
(cherry picked from commit 4dc748f14c6650cb45c7086a39af1760bfda41d2)
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/673576
Reviewed-by: John Stiles <johnstiles@google.com>
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/474618
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r-- | chromium/third_party/skia/src/sksl/ir/SkSLFunctionDefinition.cpp | 43 |
1 files changed, 26 insertions, 17 deletions
diff --git a/chromium/third_party/skia/src/sksl/ir/SkSLFunctionDefinition.cpp b/chromium/third_party/skia/src/sksl/ir/SkSLFunctionDefinition.cpp index 45695a1effe..adfe62dfd22 100644 --- a/chromium/third_party/skia/src/sksl/ir/SkSLFunctionDefinition.cpp +++ b/chromium/third_party/skia/src/sksl/ir/SkSLFunctionDefinition.cpp @@ -37,6 +37,7 @@ #include <forward_list> #include <string_view> #include <type_traits> +#include <vector> namespace SkSL { @@ -88,9 +89,29 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c bool builtin) { class Finalizer : public ProgramWriter { public: - Finalizer(const Context& context, const FunctionDeclaration& function) + Finalizer(const Context& context, const FunctionDeclaration& function, Position pos) : fContext(context) - , fFunction(function) {} + , fFunction(function) { + // Function parameters count as local variables. + for (const Variable* var : function.parameters()) { + this->addLocalVariable(var, pos); + } + } + + void addLocalVariable(const Variable* var, Position pos) { + // We count the number of slots used, but don't consider the precision of the base type. + // In practice, this reflects what GPUs actually do pretty well. (i.e., RelaxedPrecision + // math doesn't mean your variable takes less space.) We also don't attempt to reclaim + // slots at the end of a Block. + size_t prevSlotsUsed = fSlotsUsed; + fSlotsUsed = SkSafeMath::Add(fSlotsUsed, var->type().slotCount()); + // To avoid overzealous error reporting, only trigger the error at the first + // place where the stack limit is exceeded. + if (prevSlotsUsed < kVariableSlotLimit && fSlotsUsed >= kVariableSlotLimit) { + fContext.fErrors->error(pos, "variable '" + std::string(var->name()) + + "' exceeds the stack size limit"); + } + } ~Finalizer() override { SkASSERT(fBreakableLevel == 0); @@ -109,24 +130,12 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c bool visitStatement(Statement& stmt) override { switch (stmt.kind()) { case Statement::Kind::kVarDeclaration: { - // We count the number of slots used, but don't consider the precision of the - // base type. In practice, this reflects what GPUs really do pretty well. - // (i.e., RelaxedPrecision math doesn't mean your variable takes less space.) - // We also don't attempt to reclaim slots at the end of a Block. - size_t prevSlotsUsed = fSlotsUsed; const Variable& var = stmt.as<VarDeclaration>().var(); if (var.type().isOrContainsUnsizedArray()) { fContext.fErrors->error(stmt.fPosition, "unsized arrays are not permitted here"); - break; - } - fSlotsUsed = SkSafeMath::Add(fSlotsUsed, var.type().slotCount()); - // To avoid overzealous error reporting, only trigger the error at the first - // place where the stack limit is exceeded. - if (prevSlotsUsed < kVariableSlotLimit && fSlotsUsed >= kVariableSlotLimit) { - fContext.fErrors->error(stmt.fPosition, - "variable '" + std::string(var.name()) + - "' exceeds the stack size limit"); + } else { + this->addLocalVariable(&var, stmt.fPosition); } break; } @@ -219,7 +228,7 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c using INHERITED = ProgramWriter; }; - Finalizer(context, function).visitStatement(*body); + Finalizer(context, function, pos).visitStatement(*body); if (function.isMain() && ProgramConfig::IsVertex(context.fConfig->fKind)) { append_rtadjust_fixup_to_vertex_main(context, function, body->as<Block>()); } |