From 70fbd69ee6c2560ff5b483f1bb23678267dfca2b Mon Sep 17 00:00:00 2001 From: Georg Neis Date: Wed, 10 Mar 2021 09:25:45 +0100 Subject: [PATCH] [Backport] Security bug 1161847 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Partial cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/v8/v8/+/2748077: Merged: Squashed multiple commits. Merged: [const-tracking] Mark const field as mutable when reconfiguring Revision: 7535b91f7cb22274de734d5da7d0324d8653d626 Merged: [const-tracking] Fix incorrect DCHECK in MapUpdater Revision: f95db8916a731e6e5ccc0282616bc907ce06012f BUG=chromium:1161847,chromium:1185463,v8:9233 NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true R=ishell@chromium.org Change-Id: I4a34bafb3b072f2e788b47949947c76110f1b85c Reviewed-by: Igor Sheludko Commit-Queue: Georg Neis Cr-Commit-Position: refs/branch-heads/9.0@{#18} Cr-Branched-From: bd0108b4c88e0d6f2350cb79b5f363fbd02f3eb7-refs/heads/9.0.257@{#1} Cr-Branched-From: 349bcc6a075411f1a7ce2d866c3dfeefc2efa39d-refs/heads/master@{#73001} Reviewed-by: Jüri Valdmann --- chromium/v8/src/map-updater.cc | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/chromium/v8/src/map-updater.cc b/chromium/v8/src/map-updater.cc index a0ac5d3cd0a..5a7a4fc5ed3 100644 --- a/chromium/v8/src/map-updater.cc +++ b/chromium/v8/src/map-updater.cc @@ -99,6 +99,41 @@ Handle MapUpdater::ReconfigureToDataField(int descriptor, PropertyDetails old_details = old_descriptors_->GetDetails(modified_descriptor_); + // If the {descriptor} was "const" data field so far, we need to update the + // {old_map_} here, otherwise we could get the constants wrong, i.e. + // + // o.x = 1; + // change o.x's attributes to something else + // delete o.x; + // o.x = 2; + // + // could trick V8 into thinking that `o.x` is still 1 even after the second + // assignment. + // This situation is similar to what might happen with property deletion. + if (old_details.constness() == PropertyConstness::kConst && + old_details.location() == kField && + old_details.attributes() != new_attributes_) { + Handle field_type( + old_descriptors_->GetFieldType(modified_descriptor_), isolate_); + Map::GeneralizeField(isolate_, old_map_, descriptor, + PropertyConstness::kMutable, + old_details.representation(), field_type); + // The old_map_'s property must become mutable. + // Note, that the {old_map_} and {old_descriptors_} are not expected to be + // updated by the generalization if the map is already deprecated. + DCHECK_IMPLIES( + !old_map_->is_deprecated(), + PropertyConstness::kMutable == + old_descriptors_->GetDetails(modified_descriptor_).constness()); + // Although the property in the old map is marked as mutable we still + // treat it as constant when merging with the new path in transition tree. + // This is fine because up until this reconfiguration the field was + // known to be constant, so it's fair to proceed treating it as such + // during this reconfiguration session. The issue is that after the + // reconfiguration the original field might become mutable (see the delete + // example above). + } + // If property kind is not reconfigured merge the result with // representation/field type from the old descriptor. if (old_details.kind() == new_kind_) {