summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJohn Stiles <johnstiles@google.com>2023-04-12 14:56:54 -0400
committerMichael BrĂ¼ning <michael.bruning@qt.io>2023-05-02 08:59:24 +0000
commit5c93fd0f90d784bb2be6eccbc9452dbf19c70cd2 (patch)
tree87ca05021f3af7551d8f7106bfd22fbebb793f54
parent3441a3e88b0be3c7eab697e564cb72445390d65d (diff)
downloadqtwebengine-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.cpp43
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>());
}