1714531 - Assertion failure: hasValue(), at js/PropertyDescriptor.h:263

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 20210603-3350b68026ed (debug build, run with --fuzzing-safe --ion-offthread-compile=off):

e = undefined;
accDesc = { get: () => 1 };
f = { value: 1 };
g = {
  defineProperty: () => true
};
h = {};
Reflect.defineProperty(h, e, accDesc)
i = new Proxy(h, g);
Reflect.defineProperty(i, e, f);

Backtrace:

received signal SIGSEGV, Segmentation fault.
#0  0x0000555556cdbfa2 in IsCompatiblePropertyDescriptor(JSContext*, bool, JS::Handle<JS::PropertyDescriptor>, JS::Handle<mozilla::Maybe<JS::PropertyDescriptor> >, char const**) ()
#1  0x0000555556cdc89a in js::ScriptedProxyHandler::defineProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::PropertyDescriptor>, JS::ObjectOpResult&) const ()
#2  0x0000555556cd1b95 in js::Proxy::defineProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::PropertyDescriptor>, JS::ObjectOpResult&) ()
#3  0x0000555556eb3a67 in js::DefineProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::PropertyKey>, JS::Handle<JS::PropertyDescriptor>, JS::ObjectOpResult&) ()
#4  0x0000555556f961e2 in intrinsic_DefineProperty(JSContext*, unsigned int, JS::Value*) ()
#5  0x0000555556ba3151 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) ()
#6  0x0000555556ba2886 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) ()
#7  0x0000555556ba3cc1 in InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason) ()
#8  0x0000555556b97e9d in Interpret(JSContext*, js::RunState&) ()
#9  0x0000555556b8f931 in js::RunScript(JSContext*, js::RunState&) ()
#10 0x0000555556ba53f6 in js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::AbstractFramePtr, JS::MutableHandle<JS::Value>) ()
#11 0x0000555556ba5924 in js::Execute(JSContext*, JS::Handle<JSScript*>, JS::Handle<JSObject*>, JS::MutableHandle<JS::Value>) ()
#12 0x0000555556d5790f in ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::MutableHandle<JS::Value>) ()
#13 0x0000555556d57b0a in JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) ()
#14 0x0000555556a70a75 in RunFile(JSContext*, char const*, _IO_FILE*, CompileUtf8, bool) ()
#15 0x0000555556a70100 in Process(JSContext*, char const*, bool, FileKind) ()
#16 0x0000555556a179ab in Shell(JSContext*, js::cli::OptionParser*) ()
#17 0x0000555556a0f097 in main ()
rax	0x555555799687	93824994612871
rbx	0x7fffffffbc58	140737488338008
rcx	0x555558058e28	93825037340200
rdx	0x0	0
rsi	0x7ffff7105770	140737338431344
rdi	0x7ffff7104540	140737338426688
rbp	0x7fffffffbb50	140737488337744
rsp	0x7fffffffbae0	140737488337632
r8	0x7ffff7105770	140737338431344
r9	0x7ffff7f99840	140737353717824
r10	0x0	0
r11	0x0	0
r12	0x8bb79a005a8	9601292436904
r13	0x0	0
r14	0x7fffffffbb70	140737488337776
r15	0x7fffffffbc58	140737488338008
rip	0x555556cdbfa2 <IsCompatiblePropertyDescriptor(JSContext*, bool, JS::Handle<JS::PropertyDescriptor>, JS::Handle<mozilla::Maybe<JS::PropertyDescriptor> >, char const**)+1794>
=> 0x555556cdbfa2 <_ZL30IsCompatiblePropertyDescriptorP9JSContextbN2JS6HandleINS1_18PropertyDescriptorEEENS2_IN7mozilla5MaybeIS3_EEEEPPKc+1794>:	movl   $0x107,0x0
   0x555556cdbfad <_ZL30IsCompatiblePropertyDescriptorP9JSContextbN2JS6HandleINS1_18PropertyDescriptorEEENS2_IN7mozilla5MaybeIS3_EEEEPPKc+1805>:	callq  0x555556a9a9fa <abort>

Marking s-s because this assert is known to be potentially security-relevant.

Comment 1

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 2

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20210604154219-963df76dc655.
The bug appears to have been introduced in the following build range:

Start: 1514fcbf80a014f6e3b96d8815f53a9b4ebd3303 (20210530214555)
End: fafcc4a3b16a3b45f7abcc174e46bdc4a46888ca (20210531115711)
Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1514fcbf80a014f6e3b96d8815f53a9b4ebd3303&tochange=fafcc4a3b16a3b45f7abcc174e46bdc4a46888ca

Comment 3

[Iain Ireland [:iain]]

It looks like this is a pre-existing issue exposed by the assertion Tom added in bug 1712701. This code in IsCompatiblePropertyDescriptor accesses current->value() without checking current->hasValue(). I'm not sure exactly what this code is doing. It's labelled as step 4, but it doesn't seem to correspond to step 4 here. (It might correspond to a step that's been removed, since our step 5 seems to match the spec's step 4.)

Tom, any thoughts? current->value() is initialized to UndefinedValue(), so I think this is safe?

Comment 4

[Tom S. (please needinfo tschuster)]

Tom, any thoughts? current->value() is initialized to UndefinedValue(), so I think this is safe?

Yes, it should be safe. It's always undefined when missing.

Seems like our Step 4. was removed from the spec a few years ago. We can just remove it from our code as well.

Comment 5

[Tom S. (please needinfo tschuster)]

Attachment: Bug 1714531 - Update ValidateAndApplyPropertyDescriptor. r?iain

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

:evilpie, since this bug contains a bisection range, could you fill (if possible) the regressed_by field?
For more information, please visit auto_nag documentation.

Comment 7

[Tom S. (please needinfo tschuster)]

The regressed_by field is not really accurate in this case, we just uncovered a preexisting issue.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/395c63afbeaa
Update ValidateAndApplyPropertyDescriptor. r=iain

Comment 9

[Tom S. (please needinfo tschuster)]

Attachment: Bug 1714531 - Restore deleted comment

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1712701

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4445aa57a6ea
Restore deleted comment

Comment 12

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/395c63afbeaa
https://hg.mozilla.org/mozilla-central/rev/4445aa57a6ea

Comment 13

[Bugmon [:jkratzer for issues]]

Bugmon Analysis:
Verified bug as fixed on rev mozilla-central 20210608215751-74f447e17d5f.
Removing bugmon keyword as no further action possible.
Please review the bug and re-add the keyword for further analysis.

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:evilpie, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.