Fix use-after-free in SkComposeColorFilter::asFragmentProcessor

Includes a unit test to generate the exact failing scenario, and also
fixes the case where inputFP was nullptr to begin with (and we would
have tried to call nullptr->clone().

Change-Id: I5a03730d1217d9b1747e5ad2018100bc1c5f9e50
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/550714
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2022-06-17 16:30:57 -04:00 committed by SkCQ
parent 03b2e3aeee
commit 1ae33f14e1
2 changed files with 47 additions and 6 deletions

View File

@ -169,21 +169,20 @@ public:
GrFPResult asFragmentProcessor(std::unique_ptr<GrFragmentProcessor> inputFP,
GrRecordingContext* context,
const GrColorInfo& dstColorInfo) const override {
GrFragmentProcessor* originalInputFP = inputFP.get();
// Unfortunately, we need to clone the input before we know we need it. This lets us return
// the original FP if either internal color filter fails.
auto inputClone = inputFP ? inputFP->clone() : nullptr;
auto [innerSuccess, innerFP] =
fInner->asFragmentProcessor(std::move(inputFP), context, dstColorInfo);
if (!innerSuccess) {
return GrFPFailure(std::move(innerFP));
return GrFPFailure(std::move(inputClone));
}
auto [outerSuccess, outerFP] =
fOuter->asFragmentProcessor(std::move(innerFP), context, dstColorInfo);
if (!outerSuccess) {
// In the rare event that the outer FP cannot be built, we have no good way of
// separating the inputFP from the innerFP, so we need to return a cloned inputFP.
// This could hypothetically be expensive, but failure here should be extremely rare.
return GrFPFailure(originalInputFP->clone());
return GrFPFailure(std::move(inputClone));
}
return GrFPSuccess(std::move(outerFP));

View File

@ -6,12 +6,15 @@
*/
#include "include/core/SkBlendMode.h"
#include "include/core/SkCanvas.h"
#include "include/core/SkColor.h"
#include "include/core/SkColorFilter.h"
#include "include/core/SkColorSpace.h"
#include "include/core/SkRefCnt.h"
#include "include/core/SkSurface.h"
#include "include/core/SkTypes.h"
#include "include/effects/SkColorMatrix.h"
#include "include/effects/SkGradientShader.h"
#include "include/utils/SkRandom.h"
#include "src/core/SkAutoMalloc.h"
#include "src/core/SkColorFilterPriv.h"
@ -128,3 +131,42 @@ DEF_TEST(WorkingFormatFilterFlags, r) {
REPORTER_ASSERT(r, !cf->isAlphaUnchanged());
}
}
struct FailureColorFilter : public SkColorFilterBase {
skvm::Color onProgram(skvm::Builder*,
skvm::Color c,
const SkColorInfo&,
skvm::Uniforms*,
SkArenaAlloc*) const override {
return {};
}
bool onAppendStages(const SkStageRec&, bool) const override { return false; }
// Only created here, should never be flattened / unflattened.
Factory getFactory() const override { return nullptr; }
const char* getTypeName() const override { return "FailureColorFilter"; }
};
DEF_GPUTEST_FOR_ALL_CONTEXTS(ComposeFailureWithInputElision, r, ctxInfo) {
SkImageInfo info = SkImageInfo::MakeN32Premul(8, 8);
auto surface = SkSurface::MakeRenderTarget(ctxInfo.directContext(), SkBudgeted::kNo, info);
SkPaint paint;
// Install a non-trivial shader, so the color filter isn't just applied to the paint color:
const SkPoint pts[] = {{0, 0}, {100, 100}};
const SkColor colors[] = {SK_ColorWHITE, SK_ColorBLACK};
paint.setShader(SkGradientShader::MakeLinear(pts, colors, nullptr, 2, SkTileMode::kClamp));
// Our inner (first) color filter does a "blend" (kSrc) against green, *discarding* the input:
auto inner = SkColorFilters::Blend(SK_ColorGREEN, SkBlendMode::kSrc);
// The outer (second) color filter then fails to generate an FP. There are ways to do this with
// the public API, (eg: image shader with a non-invertible local matrix, wrapped in a runtime
// color filter). That's significant boilerplate, so we use a helpful "always fail" filter:
auto outer = sk_make_sp<FailureColorFilter>();
paint.setColorFilter(outer->makeComposed(inner));
// At one time, this would trigger a use-after-free / crash, when converting the paint to FPs:
surface->getCanvas()->drawPaint(paint);
}