1962309 - Eager evaluation shouldn't perform side-effectful operation during constructing previews

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[Tooru Fujisawa [:arai]]

Attachment: testcase from bug 1960745 comment 9: keylogs 1st token of every console command

derived from bug 1960745 comment #9

If eager evaluation hits any error, the Error object is passed to preview, and the preview performs side-effectful operation, that leaks the input to the web content via monkey-patched Error prototypes.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1962309 - Part 1: Do not perform side-effectful operation for eager evaluation result preview. r?nchevobbe!

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1962309 - Part 2: Add test. r?nchevobbe!

Comment 3

[Tooru Fujisawa [:arai]]

Given the following, I'm assigning sec-low, and I'll land the patch shortly.

• this requires explicit user action on DevTools console
• only the partial input or wrong input (which hits any error) gets leaked to the webpage
• once the user hits enter, the code should be evaluated in the webpage's context, and executing the monkey-patched code should be totally okay and should be the expected behavior. thus, this is a problem only because the eager evaluation isn't supposed to have side effect
• clearly less severe than bug 1960745 which has sec-moderate

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cdb7f526b187
Part 1: Do not perform side-effectful operation for eager evaluation result preview. r=nchevobbe,devtools-reviewers

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/cdb7f526b187

Comment 6

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

The patch landed in nightly and beta is affected.
:arai, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox138 to wontfix.

For more information, please visit BugBot documentation.

Comment 7

[Tooru Fujisawa [:arai]]

this patch depends on bug 1960745 patch.
So I'll prepare the uplift after that

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1962309 - Part 1: Do not perform side-effectful operation for eager evaluation result preview. r?nchevobbe!

Original Revision: https://phabricator.services.mozilla.com/D246546

Comment 9

[Tooru Fujisawa [:arai]]

Actually, this patch doesn't apply cleanly on esr128 and esr115, because of some other refactoring around the value grip and actor parameters.
Given the severity, I'm leaning toward not requesting uplift.
nchevobbe, can I have your opinion?

Comment 10

[Nicolas Chevobbe [:nchevobbe]]

(In reply to Tooru Fujisawa [:arai] from comment #9)

Actually, this patch doesn't apply cleanly on esr128 and esr115, because of some other refactoring around the value grip and actor parameters.
Given the severity, I'm leaning toward not requesting uplift.
nchevobbe, can I have your opinion?

that sounds fine to me

Comment 11

[Tooru Fujisawa [:arai]]

Thank you!