Closed Bug 178917 Opened 22 years ago Closed 1 month ago

Implement "Do JavaScript" AppleScript verb

Categories

(Camino Graveyard :: OS Integration, enhancement)

PowerPC
macOS
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: arezeeman, Assigned: jalkut)

References

Details

Attachments

(3 files, 6 obsolete files)

It would be great if Chimera could respond to a "do javascript" command from
Applescript. This is something that IE does, and in effect it gives users a
easy, programmable entry into all sorts of browser functions.

For it to be useful, the current Applescript dictionary would have to be fully
implemented (for instance, the "document" property of the Window class should
actually return something--does that warrant a separate bug report?), and maybe
expanded a little (it should be possible to target specific windows with the 
open/Get URL commands, for instance, there should be a Tab class, and it would
be great if the Window or Tab classes would return the contents of the current
page).

The syntax would be something like

do javascript <script_string> [ in <window_or_tab_reference> ]

Being able to send "do javascript" instructions to the browser would open the
door to a number of features, especially in the area of HTML forms. The
connection between the browser and the keychain could be smarter and more
customizable, and form autofill could be implimented. It might not be as fully
automatic as in Mozilla (the user might have to hit a key to trigger the script,
for instance) but that seems like a small problem. It would also be possible to
implement smarter bookmarking systems (ones that did a database-style lookup,
for instance).

It seems like not only this specific Applescript verb, but robust Applescript
support in general, is consistent with if not essential to Chimera's stated
purpose (which, if I understand correctly, is to be a lightweight browser that
showcases the strengths of the Gecko layout engine in the context of MacOS) for
these reasons: 1) Applescript is one of the most powerful and attractive
features of the Mac OS. 2) Applescript ultimately helps keep an application
small by allowing features to be offloaded to the OS and other applications. 3)
Applescript support means that a much larger subset of users can contribute to
application development--quite a few bells and whistles, and even some basic
functionality, could be provided by people like me who don't have the time or
background to wade into the source code.

It seems to me that if Chimera had full, attachable Applescript support (meaning
that scripts could be attached to buttons, keystrokes, menu items, and/or
events) it would become a contender for the title of "greatest Macintosh
software tool ever" (at least until the next big thing came along) because it
could be compact, elegant, and highly adaptable at the same time.

In the meantime, how about "do javascript"?
Dup of bug 5704?
Assignee: saari → sfraser
Component: General → OS Integration
QA Contact: winnie → petersen
Summary: Implement "do javascript" Applescript verb → Implement "Do JavaScript" AppleScript verb
Greg: yes, but this would be for chimera.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Very timely.  This is now even more applicable that javascript can now be 
called to trigger changes in a embeded Flash component. 

This would allow an applescript to trigger Chimera to trigger flash to take 
action. Tons of potential here.

Even applescript to Chimera could allow for things like voice recognition 
to voice enable your browsing experience pretty easily.

Additionally a robust dictionary for turning on/off full screen kiosk mode, 
hiding button bars etc would be great dictionary additions, some of which 
could be handled with Javascript.
Target Milestone: --- → Camino1.1
This bug seems to be the one that is not letting Web Confidential do its thing, which is allow the data kept in WC entered into the text fields. There are three options: Password, ID & Password, and Submit. This bug seems pretty old. Any chance that it can get hooked up soon? I don't have the skills to do it, appreciate all the work those that can and do.

Some background. I asked the author of WC:
Q.
What is the status of WC & Camino?
A.
As soon as Camino supports the doJavaScript AppleScript command,
I can make it work. Without it, it is impossible.

QA Contact: chrispetersen → os.integration
Target Milestone: Camino1.1 → Camino1.2
Assignee: sfraser_bugs → jalkut
CCing Mike (cause he wasn't on here), Ian, and Nick, along with myself (as potential reviewers).

Thanks a bunch for looking at this, Daniel.

cl
I have taken some baby steps towards resolving this issue.  In particular, I added support for basically forwarding any text passed via AppleScript to the "do javascript" command to the frontmost browser window, if present.

This has the effect of satisyfing the "default scenario" where a particular window/document is not specified as the target of the commands. 

Benefits:

1. This satisfies the requirements of "do javascript" for anybody who just needs to send 1-way commands to the active browser window's javascript interpreter.

Drawbacks: 

1. The return value, if  any, of the javascript execution is not propagated back to the caller.
2. The script cannot be run in any but the frontmost and active tab browser instance.

Changed files:

I'm attaching ScriptSuite.patch, which shows changes to the Camino.scriptSuite file.  These changes instruct AppleScript to dispatch this command to the "DoJavaScriptCommand" class, which I've added supporting files for.

The suporting files DoJavaScriptCommand.[mm,h] are new files to be added in "camino/src/appleevents/."
A very simple test case, but one that is very typical for me: change the URL of the frontmost browser window:

tell application "Camino"
	set targetURL to "http://www.caminobrowser.org/"
	do javascript "document.location='" & targetURL & "'"
end tell

Remove tabs and fix whitespace.
Attachment #226350 - Attachment is obsolete: true
Fix tabs and whitespace.
Attachment #226351 - Attachment is obsolete: true
General questions:
- Why isn't this opening a new window if there isn't already one?
- Why isn't this using the strategy from bug 5704 comment 19, which sounds much less hackish?
- Why are error conditions returned as strings, instead of using the AppleEvent error system (setScriptErrorNumber:/setScriptErrorString:)?

Comments:
> * Contributor(s):
> *  David Hyatt <hyatt@mozilla.org> (From GetURLCommand.mm)

I would guess that since this shares essentially no code with GetURLCommand, hyatt wouldn't be a contributor.

>  // I took this directly from GetURLCommand.mm - I assume it's the right thing to do here, too?

Yes

>  if (frontBrowserWrapper == nil)
>  {

Preferefed style is |!frontBrowserWrapper| instead of |frontBrowserWrapper == nil|, and for if/else opening braces to be on the same line as the if/else.

>    if ([userJavaScript isKindOfClass:[NSString class]] == NO)
>...
>    else

if (<condition>)
  <what to do when it's true>
else
  <what do do when it's false>

is generally clearer than the reverse order.

>      NSString* userJavaScriptURL = [@"javascript:" stringByAppendingString:userJavaScript];

stringWithFormat would probably be a clearer way to do this.

>      [frontBrowserWrapper loadURI:userJavaScriptURL referrer:nil flags:0 activate:NO allowPopups:YES];

flags should be NSLoadFlagsNone, not 0

>      NSLocalizedString(@"Camino doesn't support returning the results of 'do javascript' yet.", @"Explain Camino Do JavaScript return value shortcoming");

This is a no-op; you don't set returnedText to it. Perhaps this why you aren't seeing text returned?
(In reply to comment #13)

Hi Stuart. Thanks for taking the time to review my submitted changes. First off, let me say that I appreciate that the overall quality of this submission might be below the standard that one would normally expect for the project.  This is my first submission to the project, and I attacked this problem very much as a "solve my pet peeve" type of problem.  I appreciate that the proposed solution is not ideal. I see it as a step in the right direction, however.  Since the behavior previous to this submission is an AppleScript dictionary item that advertises functionality that doesn't exist, I thought that any move in the direction of actual functionality would be welcome.

That said, there are some great points you make, and I think at the very least my submitted changes can be considered an example of where the "real solution" might start. The fact that nothing has happened in four years of discussion to address this bug makes me think that perhaps my changes will be welcome by some users, even if it is a bit hackish. 

Responses to individual questions & comments follow:

> General questions:
> - Why isn't this opening a new window if there isn't already one?

Didn't have time and was not interested in solving this situation for my own needs. I consider this to be among the many aspects that is "to be addressed in the next iteration (probably not by me)."

> - Why isn't this using the strategy from bug 5704 comment 19, which sounds much
> less hackish?

Didn't know how or didn't have time to approach this problem in the way that is prescribed there. Again I take the defense of "it now does something where before it did nothing."

> - Why are error conditions returned as strings, instead of using the AppleEvent
> error system (setScriptErrorNumber:/setScriptErrorString:)?

No good excuse. I'm not very familiar with the Cocoa script error propagation methods. This is probably excellent criticism to have in the bug for future revisions.

> Comments:
> > * Contributor(s):
> > *  David Hyatt <hyatt@mozilla.org> (From GetURLCommand.mm)
> 
> I would guess that since this shares essentially no code with GetURLCommand,
> hyatt wouldn't be a contributor.

I prefer to err on the side of leaving credit where it is due. I used the GetURLCommand source file as a template for my implementation, and his name was the only one credited in that file.

> >  // I took this directly from GetURLCommand.mm - I assume it's the right thing to do here, too?
> 
> Yes

This seems to support the fact that I credited the only name listed in that source file as a contributor.

> >  if (frontBrowserWrapper == nil)
> >  {
> 
> Preferefed style is |!frontBrowserWrapper| instead of |frontBrowserWrapper ==
> nil|, and for if/else opening braces to be on the same line as the if/else.
> 
> >    if ([userJavaScript isKindOfClass:[NSString class]] == NO)
> >...
> >    else
> 
> if (<condition>)
>   <what to do when it's true>
> else
>   <what do do when it's false>
> 
> is generally clearer than the reverse order.
> 

Thanks. Since it's my first submission to this project and the pursuit was somewhat whimsical, I did not become very acquainted with the project's style guidelines. I apologize if this is more trouble than it's worth. I figured since I found a pretty straightforward way of moving this bug's progress forward, it was worth sharing my changes even if I was not willing to do a great amount of research into the style guidelines.

> >      NSString* userJavaScriptURL = [@"javascript:" stringByAppendingString:userJavaScript];
> 
> stringWithFormat would probably be a clearer way to do this.
> 

I guess it's just a style difference again. I'm in the habit of doing things that way so I find it quite natural.

> >      [frontBrowserWrapper loadURI:userJavaScriptURL referrer:nil flags:0 activate:NO allowPopups:YES];
> 
> flags should be NSLoadFlagsNone, not 0
> 

Thanks! 

> >      NSLocalizedString(@"Camino doesn't support returning the results of 'do javascript' yet.", @"Explain Camino Do JavaScript return value shortcoming");
> 
> This is a no-op; you don't set returnedText to it. Perhaps this why you aren't
> seeing text returned?
> 

Good catch. I don't think that was the entire cause of my not seeing string results propagate, but maybe you're right.

Bottom line: all of your criticisms are very fair so please just consider my submitted changes as a possible "stopgap fix" for what seems to be a longstanding deficiency in Camino. Personally I have waited for this functionality for years and never seen it, so I finally got tired of waiting. It sounds like others who have commented on this bug feel the same way. The changes I've submitted demonstrate a safe way of satisfying some of the requirements with very little work. I'm using it in my private copy of Camino now and enjoying it. In the spirit of the project's open source status I shared it in this bug report, but I won't be offended if you decide not to take it.

Thanks again for commenting on the changes, and I hope my response helps to clear up my motivation for submitting and the context in which I did it.

Daniel
The way patches work is that they go through one or more reviews, then a final review by a super-reviewer, then are integrated--there's definitely no reason to be offended that this wouldn't be taken as-is, because patches are rarely taken in their original form.

If you're interested in going through a few iterations here (perhaps keeping the javascript: approach, but addressing other changes), it could likely go in as an initial implementation.
Thanks, Stuart. I will make some revisions and reattach the files.  You were right about the clumsy failure to set "returnedText" being the cause for my not seeing it being propagated back to the script client.

Also I forgot to mention in response to your comment:

> - Why isn't this using the strategy from bug 5704 comment 19, which sounds much less hackish?

The reason is that bug 5704 comment 20 (right after that one) seems to rescind the strategy in favor of direct manipulation of javascript: URLs. The idea being that it more closely represents a user-level javascript invocation and thus is less frightening from a security standpoint.

Updates forthcoming... I hope you'll get a chance to review again when I update them...

Daniel
Remove the "Type" entry that I had added in the first iteration. It produces a runtime assertion and doesn't appear necessary to get the desired behavior from returning NSString results in DoJavaScriptCommand.
Attachment #226349 - Attachment is obsolete: true
This revision of the source file includes changes prompted by feedback from Stuart Morgan.

Note that the command now opens a new, blank window before proceeding to execute the user's JavaScript, if no existing front-window can be found.
Attachment #226360 - Attachment is obsolete: true
(In reply to comment #16)
> > - Why isn't this using the strategy from bug 5704 comment 19, which sounds much less hackish?
> 
> The reason is that bug 5704 comment 20 (right after that one) seems to rescind
> the strategy in favor of direct manipulation of javascript: URLs. The idea
> being that it more closely represents a user-level javascript invocation and
> thus is less frightening from a security standpoint.

It was the new window part that made it more secure though, not the approach in general.  If I read comment 24 correctly, the two methods have the same security implications when run on an existing window.  Someone who understands the JS sandboxing model should confirm that though.

This still needs some thought given to security before it lands; I don't agree with the comment in the other bug that local applications are trusted. For example, we'd want to consider whether this opens a hole whereby an application can steal Camino's keychain entries without being authorized to access them directly.
Comment on attachment 227017 [details]
DoJavaScriptCommand source to be added at camino/src/appleevents/

>  BrowserWrapper* frontBrowserWrapper = [[[mainController getFrontmostBrowserWindow] windowController] getBrowserWrapper];

Operate on BWCs, not BWs

>  if (frontBrowserWrapper == nil) {

if (!...) {

>    // NOTE: I am following the example of the "openLocation:" action in MainController's "openLocation:" IBAction method.

Remove this comment

>    // The strategy here is as described in Bugzilla 5704. Given the target window, query the interface of the 
>    // underlying Mozilla "web navigation" object, and ask it to load a javascript: URL.
>    // ---> window.content.QueryInterface(Components.interfaces.nsIInterfaceRequestor).getInterface(Components.interfaces.nsIWebNavigation).loadURI(url_from_step_2,null,null,null,null)       
>    //
>    // I'm approaching this problem incrementally, by first getting "some desirable behavior" and then working
>    // out to a full correct implementation.  For the first step, I just translate the requested javascript 
>    // command into a loadURI request on that frontmost browser wrapper.

And this one.  You might want a comment somewhere though mentioning that doing this directly through the page's JS context rather than loading a new URL may be cleaner.
Attachment #227017 - Flags: review-
Remove superfluous comments and target the BrowserWindowController instead of BrowserWrapper.
Attachment #227017 - Attachment is obsolete: true
Attachment #227021 - Flags: review?
Exactly the same as previous submission but change a conditional test to be of form "!condition" instead of "conditon == nil".
Attachment #227021 - Attachment is obsolete: true
Attachment #227024 - Flags: review?
Attachment #227021 - Flags: review?
Somehow we need to make sure that the apps with access to this AppleScript command only have more or less the same privileges as Camino's javascript: URIs do.

I don't know who we can ask, but trying all-around Mark.

Ideally, like Stuart has mentioned, we shouldn't pipe it through javascript: URIs, but use the Gecko's JS engine directly. (If anyone's curious, just go look at the implementation of the javascript: protocol.)

I think this approach is good enough for now, as long as we can ensure it's sufficiently safe too.
Håkan: I think it's very likely I'd like to continue looking at improving this situation after a "baseline" of functionality is accepted.  I like the idea of jumping straight to the JS interpreter, since it will avoid the unsightly (imho) copying of the script contents in the address bar.

Since, for the moment, everything is literally being converted to a javascript: URI, I can't fathom how the client would gain access to more than the user can do from the address bar, or a web page can do from a content-click. Good idea to weigh the question carefully, though.
(In reply to comment #23)
> Somehow we need to make sure that the apps with access to this AppleScript
> command only have more or less the same privileges as Camino's javascript: URIs
> do.

I'm not sure that's sufficient. To expand on my example concern, lets say it's possible with a DoJS command to make an app that interacts with the keychain to get a list of sites Camino has stored passwords for, uses AS to open each site in Camino, then once it's loaded and autofilled, use JS via AS to extract the password.

It's fine if a user pressing a UI element in Camino causes keychain access to a keychain item that's authorized for Camino access; I don't think it's desirable for arbitrary applications that aren't authorized for an item to be able to get access to it.

So what we need to consider is:
-Are there attacks like that that are possible
-Can anything be done to prevent them
(In reply to comment #25)
> (In reply to comment #23)
> > Somehow we need to make sure that the apps with access to this AppleScript
> > command only have more or less the same privileges as Camino's javascript: URIs
> > do.
> 
> I'm not sure that's sufficient. To expand on my example concern, lets say it's
> possible with a DoJS command to make an app that interacts with the keychain to
> get a list of sites Camino has stored passwords for, uses AS to open each site
> in Camino, then once it's loaded and autofilled, use JS via AS to extract the
> password.
> 
> It's fine if a user pressing a UI element in Camino causes keychain access to a
> keychain item that's authorized for Camino access; I don't think it's desirable
> for arbitrary applications that aren't authorized for an item to be able to get
> access to it.
> 
> So what we need to consider is:
> -Are there attacks like that that are possible
> -Can anything be done to prevent them
> 

I don't think your example would be possible for the following reasons:

1) The patch provided cannot return text back to AS
2) AFAIK, there is no way to invoke Keychain without using native obj-c code

The worst a javascript: URI can do, as far as I know, is execute JS XPCOM code, which some bookmarklets take advantage of.  

I don't know how *much* of those privileges it has, and whether it's something that can be potentially harmful if we let any app programatically "launch" such URIs.

Bz, biesi?
once it's on your machine, what's to keep an applescript from zipping up local files and emailing them to a random address? I don't see how we can prevent something already on the machine from stealing data.
So Outlook shouldn't have any protection against viruses that are already on the machine, and use it to fire off gobs of spam?

I agree that we need to be careful about the implementation of this, though I'm not sure if we should specifically prohibit things like form submit (even if we could).
I didn't read this bug, I'm just going to reply to this comment:

> The worst a javascript: URI can do, as far as I know, is execute JS XPCOM code,
> which some bookmarklets take advantage of.  

Normally javascript: URIs can not execute XPCOM code, because they inherit the privileges of the current page, but if they could then that would obviously be very bad.

bookmarklets do not usually have chrome privileges either (they may when a chrome page is loaded, like about:config, but I'm not sure about that)
Can't programs already run 'camino javascript:.....' or equivalent anyway?  So this doesn't seem like a security change to me.  Again, just responding to the comment, not the whole bug.
I'm new here so take this with a grain of salt, but it seems to me that from a security angle, all that has to be examined really is "what new mode of operation does this bring?" 

Before this change, you could already ask Camino to execute javascript on your behalf. Just ask it to open a "javscript:" URL:

tell application "Camino"
   open url "javascript:alert(document.title)"
end

The big (only?) difference with this change is that power is now granted to open a javascript URL in the context of an existing loaded page.

If there is some way via JavaScript to access elements outside of the target window's "document" object, then I think the point is moot. That would mean whatever vulnerabilities are there have already been there with the existing support. But I can't figure out how to exploit that if they are there? For instance I thought maybe I could query for "window.documents" or something to get access to the other document objects.  Can anybody spot a way to do that?

If that is locked up tight, then the very valid question is reduced to: "Are there new vulnerabilities introduced by granting outside access to an *existing* document's DOM?"

I think the security conversation can probably be focused on that specific question.
> The big (only?) difference with this change is that power is now granted to
> open a javascript URL in the context of an existing loaded page.

That would be a problem, actually.  See bug 298255.  That bug is why javascript: URLs loaded from other apps do NOT run in the context of the current document.
(In reply to comment #26)
> I don't think your example would be possible for the following reasons:
> 
> 1) The patch provided cannot return text back to AS
> 2) AFAIK, there is no way to invoke Keychain without using native obj-c code

1) is only a temporary drawback, presumably, and isn't necessary anyway. There are all kinds of ways it could be used once available in the JS context--screenshots of an alert(password), setting the document.location to the URL of a CGI passing the password, etc.
2) is irrelevant; Camino does it for you.  Let me try to make the steps more clear:

-PasswordStealer.app tells Camino to open Slashdot.org
-PasswordStealer.app waits until it's finished loading.
-Meanwhile Camino loads Slashdot.org, finds that I have a keychain entry for it, and fills in my username and password, all automagically, as it always does
-PasswordStealer extracts the value of the password field via JS, and does something nefarious with it.

Now repeat with a bank, or some other site you really care about.  It will be possible, and it will be possible precisely because it will run in the context of an existing page (where Camino has helpfully filled in a value from the keychain).

Yes, there is the argument that if you have a malicious local app you are sunk anyway, but the entire point of the Keychain's access control system is to prevent one local app from accessing the secure data of another app when it's not authorized to do so.  Isn't that the whole point of using the Keychain instead of storing website passwords in a flat file?
This is a very interesting angle on the vulnerability. To test the concept I ran some example javascript against a password-populated page, and discovered that there is indeed a problem.  

For instance, go to the following URL:

http://www.red-sweater.com/forums/login.php

Now type something "secret" into the password edit box. This simulates (I assume) what Camino would do with a keychain-fetched password.  Now run the following JS from the address bar:

javascript:alert(document.forms[0].elements[4].value)

You should see your secret.

The vulnerability is present in Safari as well.
this bug (secretly) asks for parity with MacIE or NS4's applescript supprt. I seem to recall being able to get the entire page contents from applescript in those older browsers. Can someone check their dictonaries? If so, users are already soaking in this problem...
Users are only soaking in it if they currently store all their passwords in NS4 or IE too, which I doubt is the case for most people.  I don't think Camino should emulate MacIE security holes any more than Firefox should emulate WinIE security holes.

Is there any way to run these scripts in a slightly different security context that would disallow things like reading the value of password fields?  (Frankly, the fact that I can go to a window with a bulleted-out password field and type some JS in the location bar and be shown the password feels like a problem too--there's a reason you can't "copy" from password fields.)
Attachment #227024 - Flags: review? → review?(stuart.morgan)
Comment on attachment 227024 [details]
DoJavaScriptCommand source to be added at camino/src/appleevents/

Clearing review flag; there's not really any reason to discuss specifics of the patch until the security issue has been resolved.

Since my primary concern here is the password theft--could someone weigh in on the possibility of a JS context that would prevent access to password field values and/or other scary things?
Attachment #227024 - Flags: review?(stuart.morgan)
Target Milestone: Camino1.6 → ---
Blocks: 380580
Blocks: 156078
No longer blocks: 380580
Now that we have ways of executing AS from within Camino (toolbar items, when they land), it should be safe to implement a "do javascript" that will only work if it's called from within Camino, therefore not breaking the Keychain semantic.  We discussed this on IRC.  I'm not certain whether such a condition is possible to implement, but I'll look into it.  If it is, that gives us one way to do this.  We might use a similar system to expose the DOM also, or we might just let people do that via JS.

On the other hand, a security-conscious JS context would be more powerful, since any app could execute non-risky JS.  I'm sure it's on a much longer timeframe, though, if it's even possible.  If we can do what I'm imagining above, we can probably do it all in Camino code; a JS context would surely be in Core.
(In reply to Stuart Morgan from comment #36)
> (Frankly, the fact that I can go to a window with a bulleted-out
> password field and type some JS in the location bar and be shown the
> password feels like a problem too--there's a reason you can't "copy" from
> password fields.)

http://hints.macworld.com/article.php?story=20120926103722471 :P  There's apparently a whole market for Safari and Chrome extensions that do that automatically :P
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: