Skip to content

Commit c925fd0

Browse files
nhspritejianliang00
authored andcommitted
[BugFix] Avoid UAF when resolving unified pipeline target node
- Store the unified pixel pipeline target as an element id instead of a raw Element pointer in PipelineOptions. - Resolve the element lazily from the node manager and abort safely when the target node has already been removed, which prevents use-after-free during style resolution. - Reset the cached target node id after resolve requests are cleared and keep the pipeline context unit test aligned with the new behavior.
1 parent 6da2835 commit c925fd0

7 files changed

Lines changed: 25 additions & 10 deletions

File tree

core/public/pipeline_option.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <sys/types.h>
99

10+
#include <cstdint>
1011
#include <sstream>
1112
#include <string>
1213
#include <thread>
@@ -75,6 +76,7 @@ struct PipelineOptions {
7576
static const int32_t kRecycleTemplateBundle = 0x01 << 2;
7677
static const int32_t kProcessLayoutWithoutUIFlush = 0x01 << 3;
7778
static const int32_t kRenderForRecreateEngine = 0x01 << 4;
79+
static constexpr int32_t kInvalidTargetNodeId = 0;
7880

7981
// TODO(kechenglong): impl ToLepusValue here.
8082
// Default constructor that generates a unique PipelineID
@@ -179,7 +181,7 @@ struct PipelineOptions {
179181
bool resolve_requested{false};
180182
bool layout_requested{false};
181183
bool flush_ui_requested{false};
182-
Element* target_node{nullptr};
184+
int32_t target_node{kInvalidTargetNodeId};
183185
// Whether the current template has been reloaded.
184186
bool reload{false};
185187
const PipelineVersion* version{nullptr};

core/renderer/dom/element_manager.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,8 @@ void ElementManager::OnFinishUpdateProps(
698698
// redundant logic here.
699699
if (options->enable_unified_pixel_pipeline) {
700700
options->resolve_requested = true;
701-
options->target_node = target_node;
701+
options->target_node = target_node ? target_node->impl_id()
702+
: PipelineOptions::kInvalidTargetNodeId;
702703
} else {
703704
OnPatchFinish(options, target_node);
704705
}
@@ -1364,9 +1365,17 @@ void ElementManager::OnPatchFinish(std::shared_ptr<PipelineOptions> &option,
13641365
}
13651366

13661367
void ElementManager::ResolveStyle(std::shared_ptr<PipelineOptions> &option,
1367-
Element *element) {
1368-
if (element == nullptr) {
1368+
int32_t target_node) {
1369+
Element *element = nullptr;
1370+
if (target_node == PipelineOptions::kInvalidTargetNodeId) {
13691371
element = static_cast<Element *>(root());
1372+
} else {
1373+
element = node_manager()->Get(target_node);
1374+
if (element == nullptr) {
1375+
LOGE("ElementManager::ResolveStyle failed since target node is gone, id:"
1376+
<< target_node);
1377+
return;
1378+
}
13701379
}
13711380
if (!element) {
13721381
LOGE("ElementManager::OnPatchFinish failed since element is nullptr.");

core/renderer/dom/element_manager.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -981,8 +981,9 @@ class ElementManager : public ElementContextDelegate,
981981
* Used By RunPixelPipeline Process.
982982
*
983983
*/
984-
void ResolveStyle(std::shared_ptr<PipelineOptions> &option,
985-
Element *root = nullptr);
984+
void ResolveStyle(
985+
std::shared_ptr<PipelineOptions> &option,
986+
int32_t target_node = PipelineOptions::kInvalidTargetNodeId);
986987

987988
/**
988989
* Generate ID for element

core/renderer/dom/fiber/fiber_element.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3322,7 +3322,7 @@ void FiberElement::SetNativeProps(
33223322
// redundant logic here.
33233323
if (pipeline_options->enable_unified_pixel_pipeline) {
33243324
pipeline_options->resolve_requested = true;
3325-
pipeline_options->target_node = this;
3325+
pipeline_options->target_node = impl_id();
33263326
} else {
33273327
element_manager()->OnPatchFinish(pipeline_options, this);
33283328
}
@@ -3709,7 +3709,7 @@ void FiberElement::UpdateCSSVariable(
37093709
// redundant logic here.
37103710
if (pipeline_option->enable_unified_pixel_pipeline) {
37113711
pipeline_option->resolve_requested = true;
3712-
pipeline_option->target_node = this;
3712+
pipeline_option->target_node = impl_id();
37133713
} else {
37143714
element_manager()->OnPatchFinish(pipeline_option, this);
37153715
}

core/renderer/pipeline/pipeline_context.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ void PipelineContext::ResetResolveRequested() {
116116
return;
117117
}
118118
options_->resolve_requested = false;
119-
options_->target_node = nullptr;
119+
options_->target_node = PipelineOptions::kInvalidTargetNodeId;
120120
}
121121

122122
void PipelineContext::ResetLayoutRequested() {

core/renderer/pipeline/pipeline_context_unittest.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,10 @@ TEST_F(PipelineContextTest, TestPipelineContextResolve) {
7171
EXPECT_FALSE(context->IsResolveRequested());
7272
context->RequestResolve();
7373
EXPECT_TRUE(context->IsResolveRequested());
74+
options_->target_node = 42;
7475
context->ResetResolveRequested();
7576
EXPECT_FALSE(context->IsResolveRequested());
77+
EXPECT_EQ(options_->target_node, PipelineOptions::kInvalidTargetNodeId);
7678
}
7779

7880
TEST_F(PipelineContextTest, TestPipelineContextLayout) {

core/runtime/lepus/bindings/renderer_functions.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5149,7 +5149,8 @@ RENDERER_FUNCTION_CC(FiberFlushElementTree) {
51495149
// redundant logic here.
51505150
if (current_option->enable_unified_pixel_pipeline) {
51515151
current_option->resolve_requested = true;
5152-
current_option->target_node = element;
5152+
current_option->target_node =
5153+
element ? element->impl_id() : PipelineOptions::kInvalidTargetNodeId;
51535154
current_option->need_trigger_data_updated_ = trigger_data_updated;
51545155
} else {
51555156
self->page_proxy()->element_manager()->OnPatchFinish(current_option,

0 commit comments

Comments
 (0)