Closed Bug 1498739 Opened 6 years ago Closed 5 years ago

userScripts.setScriptAPIs's cloning of parameter/return value reduces its usefulness for practical applications

Categories

(WebExtensions :: General, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1509339

People

(Reporter: robwu, Assigned: rpl)

References

Details

Attachments

(2 files, 6 obsolete files)

Bug 1437864 allows extensions to register a custom API for user scripts.

The logic is at [1], and performs a structured clone of parameters and return values. Functions are only cloned (via exportFunction) if they are passed as an argument. Function properties on objects are not cloned (due to the lack of cloneFunctions).

This limitation makes it impractical to create a proper user script add-on. For example, GM_xmlHttpRequest is an API function for user scripts that takes one object and several callback functions are properties on the object [2]. Test case: setScriptAPIs-identities.zip

Another issue with cloning is that object identities are lost. Test case: setScriptAPIs-function-param.zip

(and obviously, cloning also prevents non-cloneable objects such as DOM objects from being passed to the function).


I think that these issues can be resolved by not cloning at all, without compromising on security.

The following principals are involved:
- apiScope runs API scripts and are privileged, trusted by the extension.
- userScriptScope runs user scripts, which are not trusted by the extension.
- the page where these scripts run is not trusted by either script type.

These trust levels are already reflected by the principals:
- apiScope is:         [Expanded Principal [http://example.com, moz-extension://...]]
- userScriptScope is:  [Expanded Principal [http://example.com]]
- userScriptScope.document.nodePrincipal is http://example.com

Because the upper scope subsumes the bottom ones, the top ones can freely read values from the lower principals. E.g. apiScope can read objects from userScriptScope and call functions from it.
The opposite is not possible by default.
apiScope has access to export helpers (cloneInto/exportFunction), so it can export objects to lower principals if desired.

Xrays are enabled for both the apiScope and the userScriptScope sandboxes, so apiScope will have Xray vision [3] for objects from userScriptScope, and thus not accidentally trigger a function or accessor (getter).



[1] https://searchfox.org/mozilla-central/rev/28fe656de6a022d1fc01bbd3811023fca99cf223/toolkit/components/extensions/ExtensionContent.jsm#667-720
[2] https://wiki.greasespot.net/GM.xmlHttpRequest
[3] https://developer.mozilla.org/en-US/docs/Mozilla/Tech/Xray_vision
STR:

1. Load attached extension in Nightly (64).
2. Visit example.com.
3. Open the tab's devtools and look at the console.

Expected:
- No errors, followed by: "a = string in array, b = string in array"

Actual:
- Console has the following two errors:
Assertion failed: Expected input parameters to be identical to each other apiscript.js:3:9
Assertion failed: Expected a to be b userscript.js:6:1
STR:

1. Load attached extension (setScriptAPIs-function-param.zip) in Nightly (64).
2. Visit example.com.
3. Open the tab's devtools and look at the console.

Expected:
- No errors, three messages:
"API script will call the function parameters"
"Called callback (param 1)"
"Called callback (param 2)"

Actual:
- The first two messages appear as expected, but the last one does not appear. The following error is shown instead:
"TypeError: "params[1].someFunctionProperty is not a function"
Attachment #9017524 - Attachment is obsolete: true
Priority: -- → P2
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
Attachment #9023742 - Attachment is obsolete: true
Comment on attachment 9023742 [details]
Bug 1498739 - Prevent userScript from being able to redefine globals needed by the apiScript methods. r?robwu,zombie!

Marking as obsolete (because this patch has been dropped and it is not part of this issue anymore).
Comment on attachment 9023742 [details]
Bug 1498739 - Prevent userScript from being able to redefine globals needed by the apiScript methods. r?robwu,zombie!

Bug 1468579 will address the needs that were originally meant to be solved by this dropped patch.
Attachment #9020778 - Attachment is obsolete: true
Attachment #9017213 - Attachment is obsolete: true
Attachment #9020779 - Attachment is obsolete: true
Attachment #9020780 - Attachment is obsolete: true
I'm closing this as a duplicate of Bug 1509339: 

- the accepted revision D10061 has been moved into Bug 1509339
- all the changes discussed on closed phabricator revisions has been included in the new patches attached to Bug 1509339
- Bug 1509339 covers the issue described by this bug (along with implementing the changes to the userScript API, as we agreed during the last round of reviews on the closed phabricator revisions).
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: