293933 - Venkman Interactive Session Evaluation Object "EvalError" in Firefox builds

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[And Nall]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4

After upgrading to Firefox 1.0.4, Venkman gives me an "EvalError: function eval
must be called directly and not by way of a function of another name." (Even on
something as simple as "1 + 1")

Reproducible: Always

Steps to Reproduce:
1. Start Venkman
2. Open Interactive Session
3. Evaluate 1 + 1

Actual Results:  
EvalError: function eval must be called directly and not by way of a function of
another name. @ <x-jsd:interactive-session>

Expected Results:  
Returned the number 2

Comment 1

[Mike Shaver (:shaver emeritus)]

Looks like Venkman will need to change to using something like a Script object
to evaluate the expression in whatever other context it is trying to manipulate.

Comment 2

[Bob Clary [:bc] (inactive)]

I'm running Venkman 0.9.85 and 1+1 works for me as do some other interactive
evaluations. eval('1+1') even works. What version of Venkman are you running? 

Comment 3

[And Nall]

(In reply to comment #2)
> I'm running Venkman 0.9.85 and 1+1 works for me as do some other interactive
> evaluations. eval('1+1') even works. What version of Venkman are you running? 

Okay, I am a moron.
I was using 0.9.85 and it wasn't working, but I just uninstalled the extension
and re-installed and now it works.

So should I resolve INVALID or will you?

Comment 4

[And Nall]

I actually missed a step in my bug report.

You must set a page as the eval object first, then the error starts occouring.

Comment 5

[Bob Clary [:bc] (inactive)]

Ok, I can confirm that!

Comment 6

[Brendan Eich [:brendan]]

Sorry, we do not let chrome callers to eval run with the eval method's scoped
principals any longer.  Maybe we should, because that's not as scary as the
reverse case.  It's still scary.

/be

Comment 7

[[e-mail]]

Attachment: Fixes my trunk suite build

Comment 8

[Brendan Eich [:brendan]]

Comment on attachment 183503 [details] [diff] [review]
Fixes my trunk suite build

Oh, I see -- the problem is the form of eval where a scope 2nd param is passed.

Instead of using Script, just use __o.eval(__s).

/be

Comment 9

[Brendan Eich [:brendan]]

Neil says my suggestion is stopped cold by the new belt-and-braces error case in
obj_eval -- d'oh.

But how does his clever __o.Script(__s).exec(__o) work around the same check in
script_exec?  The caller is still chrome (venkman), and the exec method being
called should be parented by the __o window, which has content principals.

/be

Comment 10

[Brendan Eich [:brendan]]

Something's wrong here.  If there's a secure workaround for the eval check, then
the eval check is not necessary.

/be

Comment 11

[Brendan Eich [:brendan]]

*** Bug 298851 has been marked as a duplicate of this bug. ***

Comment 12

[Marcwolf]

I see lots of interested people.. But can you developers FIX the thing...

Is this a bug.. or a new security design feature?

It seems you have already isolated the code that is causing the problem.. 
the 'Belts and Buckle' area.. so can you remove it or what is the proceedure.

Dave

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The bug is still present in current trunk builds of Firefox and Venkman 0.9.85.
It's real a hassle! You aren't able to easily debug javascript code during the
development of web pages. Since bug 296102 is fixed now, more people will start
using Venkman for Firefox. So please finish this fix ASAP. Thanks.

Comment 14

[Brendan Eich [:brendan]]

Attachment: fix

I stand by comment 10.	There is no security hole if we force the scope object
of the eval to be the parent of the eval function object being invoked, when
the nominal scope object's principals don't match that function object's
principals.

Consider Neil's patch:

	 if (__o)
	 {
-	     rv = eval(__s, __o);
+	     rv = __o.Script(__s).exec(__o);
	 }

__o is a window object.  If governing security policies allow access from the
subject to __o, then __o.eval(__s) should be allowed.  The only way with web JS
for access to be allowed (short of signed scripts, I forget too much about them
and the way they work, or don't work, nowadays) is if caller subject and __o's
principals are identical (the same JSPrincipals struct).

If principals differ, but access was allowed, then the subject must be chrome
-- venkman, in this case.  Instead of an error, in this case the eval object's
principals, which are __o's principals, should be used instead of the caller's.


The case with script_exec is not different, but I don't propose to back out the
"belt and braces" INDIRECT_CALL error I added there.  No one uses Script
objects in ways that broke due to that error case being added.	I'm sorry I
broke cases that matter to Venkman users, and I'd like to fix that now for
1.8b3/DP1.1a2.

/be

Comment 15

[Mike Shaver (:shaver emeritus)]

Comment on attachment 188389 [details] [diff] [review]
fix

r=shaver

Comment 16

[Brendan Eich [:brendan]]

I said to shaver on IRC that I distrust Script objects anyway, since they
separate compilation from execution, allowing malicious content (if other
protections such as XPCNativeWrapper are missing) to trick chrome that calls a
nullary function on a content object into running arbitrary script in the
context of the caller.

That last bit, dynamic scope (in the caller's context), is the real killer. 
With eval, it's constrained by compilation and execution being coupled in one
call, using the same implicit and explicit parameters.  With Script objects,
it's not.  So short of ripping out Script, which we'll get to, I'm leaving the
INDIRECT_CALL error in script_exec.

Fix checked in.

/be

Comment 17

[Marcwolf]

So.. What does this tell us in layman's terms..

a.  Its too hard and it won't be fixed.
b.  We don't feel that script programmers should do this anyway?

Hey guys..  Most of us know that the EVAL is a very powerful command and is 
used in situations where a simple javascript commands can not.
I'm not sure what Venkman is.. I came into this because when I posed the error 
I was getting - it was deemed  'The Venkman Issue' and lumped in.

It still does not tell me is this page will ever run under Firefox 1.0.2+

http://www.mattkruse.com/javascript/dragiframe/index.html

Awaiting any information

Dave

Comment 18

[Brendan Eich [:brendan]]

(In reply to comment #17)
> So.. What does this tell us in layman's terms..

First, when a bug is marked FIXED, it means the person who marked it thinks he
has in fact fixed it, so you shouldn't be worried that we are trying to sweep
anything under the rug.  If a bug is considered a feature, it will be marked
WONTFIX instead.

Second, thanks for the http://www.mattkruse.com/javascript/dragiframe/index.html
link -- it shows an example that doesn't work, and didn't work even before the
indirect-call error change that I made to eval went in for 1.0.4 (not 1.0.2). 
I've filed a new bug on the problem, bug 299791.  It is not the same as this
problem (you were given a bum steer).  This problem should be fixed now (someone
will confirm by testing tomorrow's build, I'm confident).

/be

Comment 19

[Marcwolf]

Many Thanks Brendan for the explanation.

We use the example in our pages together with a 'back-channel' to the server to 
give our web application additional functionality. 

For instance you are on the main page and you pop up a smaller page which you 
can drag around the browser area and select an item on it, then this item can 
be transfered to the main page using the parent/child iframe functionalty.

We strive from cross browser compatibility with our applications, and take 
pride in stretching the envelope of traditional browser applications.
Our goal is to make a browser application look and work exactly like a GUI 
application in a client/server environment.

Many Thanks

Dave 
 

Comment 20

[shutdown]

As a result of changes in this bug, Interactive Session Evaluation
uses wrong scope when evaluating an expression. See bug 299797.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b2) Gecko/20050705
Firefox/1.0+