indirect eval prevention code can be circumvented by eval(s,o)

RESOLVED FIXED

Status

()

Core
JavaScript Engine
--
major
RESOLVED FIXED
10 years ago
5 years ago

People

(Reporter: Brian Crowder, Assigned: Brian Crowder)

Tracking

unspecified
Points:
---
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:high])

Attachments

(1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 330250 [details] [diff] [review]
return if scopeobj

in obj_eval, where we report BAD_INDIRECT_CALL, we don't return JS_FALSE in the case where there is a scopeobj.  Worse, this whole business can be circumvented entirely by using the eval(o,s) form being removed in bug 442333.  If we restore that functionality, we must remain conscious of the indirect eval issue.

mrbkap:  Think I should go ahead and move this paragraph below the eval(o,s) handling stuff, for future-proofing?  Or move -that- paragraph up?
Attachment #330250 - Flags: review?(mrbkap)
(Assignee)

Comment 1

10 years ago
Comment on attachment 330250 [details] [diff] [review]
return if scopeobj

Nevermind, this is bogus.  I hadn't realized how tricksy JS_ReportError* and friends were.  This apparently returns FALSE in the JSREPORT_ERROR case!  You learn something new every day.  :)  The eval(o,s) issue should still be addressed, I'll morph the bug.
Attachment #330250 - Attachment is obsolete: true
Attachment #330250 - Flags: review?(mrbkap)
(Assignee)

Updated

10 years ago
Summary: indirect eval prevention code has a bug → indirect eval prevention code can be circumvented by eval(o,s)
Whiteboard: [sg:high]
What is this bug about now?

/be
Summary: indirect eval prevention code can be circumvented by eval(o,s) → indirect eval prevention code can be circumvented by eval(s,o)
(Assignee)

Comment 3

10 years ago
Brendan:  The indirect eval check in jsobj.cpp happens before the scopeobj replacement in the eval(s, o) case.
(Assignee)

Comment 4

9 years ago
This needs a better approach, and we need to be cautious of breaking extensions with the rephrasing I propose.  I'll try to talk with mrbkap asap about the right thing here.
(Assignee)

Comment 5

9 years ago
moz_bug_r_a4:  Can you beat on the patch in bug 446026 to see if it still presents the same issue?

Comment 6

9 years ago
(In reply to comment #3)
> Brendan:  The indirect eval check in jsobj.cpp happens before the scopeobj
> replacement in the eval(s, o) case.

I'm not sure what the problem is.  It seems to me that the indirect eval check
is not affected by the second argument.  Is there something I am missing?
(Assignee)

Comment 7

9 years ago
This is currently fixed by the removal of eval(s,o) in bug 442333.  I'll make sure I deal with the security issues revealed here in bug 446026.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 8

9 years ago
AIUI, this bug is fixed, as long as we don't have to restore support for the second argument to eval(). If this bug isn't actually fixed, or we must support the second argument to eval, please reopen.

Comment 9

9 years ago
crowder, gotatest?
Flags: in-testsuite?
Flags: in-litmus-
(Assignee)

Comment 10

9 years ago
The other bug has one, for eval(s, o) removal.  I don't have a test for the specific security issue here, though eyeballing the code still leaves me concerned.  Based on comment #6, perhaps I was overreacting.

Updated

9 years ago
Depends on: 446026

Updated

9 years ago
Flags: in-testsuite? → in-testsuite-
Group: core-security
You need to log in before you can comment on or make changes to this bug.