Skip to content

Commit 82304e1

Browse files
johnstiles-googlemibrunin
authored andcommitted
[Backport] CVE-2023-2136: Integer overflow in Skia (2/2)
Cherry-pick of patch originally reviewed on https://skia-review.googlesource.com/c/skia/+/673577: Enforce size limits on struct and array declarations. 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 tests/sksl/shared/Ossfuzz37900.* Not present in 108, skipped. src/sksl/ir/SkSLType.cpp: - Conflicting includes - MakeStructType(): - Conflicting function signature - context isn't a parameter, used ThreadContext::Context() directly. This improves error reporting by more clearly attaching the error message to the oversized type. Bug: chromium:1432603 Change-Id: I26511f08aff22072cf4913abf7be2c49940a732c Reviewed-on: https://skia-review.googlesource.com/c/skia/+/671377 Commit-Queue: John Stiles <[email protected]> (cherry picked from commit 1cbd33ecd73523f8d4bf88e9c5576303b39e5556) Reviewed-on: https://skia-review.googlesource.com/c/skia/+/673577 Reviewed-by: John Stiles <[email protected]> Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/474619 Reviewed-by: Allan Sandfeld Jensen <[email protected]>
1 parent 5c93fd0 commit 82304e1

File tree

2 files changed

+19
-5
lines changed

2 files changed

+19
-5
lines changed

chromium/third_party/skia/src/sksl/dsl/DSLType.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ DSLExpression DSLType::Construct(DSLType type, SkSpan<DSLExpression> argArray) {
266266

267267
DSLType Array(const DSLType& base, int count, Position pos) {
268268
count = base.skslType().convertArraySize(ThreadContext::Context(), pos,
269-
DSLExpression(count, pos).release());
269+
DSLExpression(count, pos).release());
270270
if (!count) {
271271
return DSLType(kPoison_Type);
272272
}
@@ -278,7 +278,7 @@ DSLType UnsizedArray(const DSLType& base, Position pos) {
278278
return DSLType(kPoison_Type);
279279
}
280280
return ThreadContext::SymbolTable()->addArrayDimension(&base.skslType(),
281-
SkSL::Type::kUnsizedArray);
281+
SkSL::Type::kUnsizedArray);
282282
}
283283

284284
DSLType Struct(std::string_view name, SkSpan<DSLField> fields, Position pos) {

chromium/third_party/skia/src/sksl/ir/SkSLType.cpp

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
#include "include/private/SkTFitsIn.h"
1313
#include "include/sksl/SkSLErrorReporter.h"
1414
#include "src/core/SkMathPriv.h"
15+
#include "src/core/SkSafeMath.h"
1516
#include "src/sksl/SkSLBuiltinTypes.h"
1617
#include "src/sksl/SkSLConstantFolder.h"
1718
#include "src/sksl/SkSLContext.h"
1819
#include "src/sksl/SkSLProgramSettings.h"
20+
#include "src/sksl/SkSLThreadContext.h"
1921
#include "src/sksl/ir/SkSLConstructorArrayCast.h"
2022
#include "src/sksl/ir/SkSLConstructorCompoundCast.h"
2123
#include "src/sksl/ir/SkSLConstructorScalarCast.h"
@@ -648,6 +650,17 @@ std::unique_ptr<Type> Type::MakeScalarType(std::string_view name, const char* ab
648650

649651
std::unique_ptr<Type> Type::MakeStructType(Position pos, std::string_view name,
650652
std::vector<Field> fields, bool interfaceBlock) {
653+
size_t slots = 0;
654+
for (const Field& field : fields) {
655+
if (field.fType->isUnsizedArray()) {
656+
continue;
657+
}
658+
slots = SkSafeMath::Add(slots, field.fType->slotCount());
659+
if (slots >= kVariableSlotLimit) {
660+
ThreadContext::Context().fErrors->error(pos, "struct is too large");
661+
break;
662+
}
663+
}
651664
return std::make_unique<StructType>(pos, name, std::move(fields), interfaceBlock);
652665
}
653666

@@ -1120,8 +1133,9 @@ bool Type::checkIfUsableInArray(const Context& context, Position arrayPos) const
11201133
return true;
11211134
}
11221135

1123-
SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos,
1124-
std::unique_ptr<Expression> size) const {
1136+
SKSL_INT Type::convertArraySize(const Context& context,
1137+
Position arrayPos,
1138+
std::unique_ptr<Expression> size) const {
11251139
size = context.fTypes.fInt->coerceExpression(std::move(size), context);
11261140
if (!size) {
11271141
return 0;
@@ -1138,7 +1152,7 @@ SKSL_INT Type::convertArraySize(const Context& context, Position arrayPos,
11381152
context.fErrors->error(size->fPosition, "array size must be positive");
11391153
return 0;
11401154
}
1141-
if (!SkTFitsIn<int32_t>(count)) {
1155+
if (SkSafeMath::Mul(this->slotCount(), count) > kVariableSlotLimit) {
11421156
context.fErrors->error(size->fPosition, "array size is too large");
11431157
return 0;
11441158
}

0 commit comments

Comments
 (0)