Modifying Navigator Object Topic is solved

Talk about code development, features, specific bugs, enhancements, patches, and similar things.
Forum rules
Please keep everything here strictly on-topic.
This board is meant for Pale Moon source code development related subjects only like code snippets, patches, specific bugs, git, the repositories, etc.

This is not for tech support! Please do not post tech support questions in the "Development" board!
Please make sure not to use this board for support questions. Please post issues with specific websites, extensions, etc. in the relevant boards for those topics.

Please keep things on-topic as this forum will be used for reference for Pale Moon development. Expect topics that aren't relevant as such to be moved or deleted.
User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-25, 17:26

Yep. It's the entire purpose of this spec. ADPC is straight awful, isn't it? Practically begging for abuse.

For clarification: my implementation only uses a doorhanger if the request comes from HTTP headers or <link> tags at present; the javascript function shows a dialog window instead. Also, all this has to be in the top level document; the spec says that embedded windows/iframes are ignored.

New Tobin Paradigm

Re: Modifying Navigator Object

Unread post by New Tobin Paradigm » 2021-06-25, 18:56

So the code that triggers the doorhanger is chrome code used to watch and interpret the trigger but web pages cannot trigger a doorhanger in and of its self? What I am asking is do we have a security issue here?

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-25, 19:13

It's all in this file. Everything relevant is above line ~200.

The content code just passes the strings that are shown in the doorhanger (and their ids). There's also a function that's supposed to trigger it (but as I said I use a dialog for the js call instead of a doorhanger) which is exposed to content via navigator, but is also technically chrome code wrapped in a content-accessible promise. Everything else is chrome code. Pale Moon is not leaking the PopupNotifications object to content. Though I'd say this extension probably weakens its defenses (especially currently), and will certainly not be submitted for inclusion on the addons list.

New Tobin Paradigm

Re: Modifying Navigator Object

Unread post by New Tobin Paradigm » 2021-06-25, 20:09

Then you should put it away before some jerkoff uses it.

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-25, 20:47

Basically the reason to keep it up is so nobody posts a "Why haven't you..." topic about it.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35402
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Modifying Navigator Object

Unread post by Moonchild » 2021-06-25, 22:44

So, begs the question, why are you spending all this time trying to break Xrays when you could use that time offering patches to implement it in the navigator interface in the core, instead?
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-26, 00:11

Because this is one standard I genuinely don't want to be implemented directly in the browser. It's kind of a preventative extension, rather than an assistive one.

User avatar
Tharthan
Board Warrior
Board Warrior
Posts: 1409
Joined: 2019-05-20, 20:07
Location: New England

Re: Modifying Navigator Object

Unread post by Tharthan » 2021-06-26, 01:19

If the implementation of a particular "new standard" somehow required code that was a genuine security risk or a user privacy risk, couldn't a footnote be put in the release notes and on the Pale Moon website indicating "particular standard X implemented insofar as was both functional and sensible so as to not make the browser less secure" or something like that?

That obviously doesn't look anywhere near as good as "standard implemented entirely", but it may be the only sensible choice if something that is outright malicious to users becomes declared a standard.
"This is a war against individuality and intelligence. Only thing we can do is stand strong."adesh, 9 January 2020

"I used to think I was a grumpy old man, but I don't hold a candle compared to Tharthan."Cassette, 9 September 2020

Image

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-26, 01:28

In this case, the core of the standard revolves around showing page/script-provided strings in chrome widgets, which are only guaranteed to be accurate descriptions of their resulting behavior because an unenforceable law says they must be. The chance for abuse is unimaginable, unless you just did a blanket "allow all" or "block all" as the only two choices rather than ever actually prompting the user, in which case it's basically DNT with extra steps.

User avatar
seadragon
Hobby Astronomer
Hobby Astronomer
Posts: 20
Joined: 2021-06-18, 04:39

Re: Modifying Navigator Object

Unread post by seadragon » 2021-06-26, 07:03

I guess you'd better stop putting efforts into it until this so-called standard stabilizes. 1. Although they make their website look like a web standard, that is an "Unofficial Draft" and may change at any time; 2. No user will actually benefit from your extension until ADPC becomes a real standard and other browsers start to implement it; 3. While not beneficial, these premature, so-called privacy features can be used as new vector for fingerprinting, thus bad for your users. (That's why I stop sending "Do Not Track" headers.)

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35402
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Modifying Navigator Object

Unread post by Moonchild » 2021-06-26, 12:54

RealityRipple wrote:
2021-06-26, 00:11
Because this is one standard I genuinely don't want to be implemented directly in the browser. It's kind of a preventative extension, rather than an assistive one.
Then why are you even working on an extension?
If this is harmful, then we don't want it by ANY means.

As you said the potential for abuse (including phishing and misleading users into thinking they are giving permission for A while they are actually giving permission for something else) is ridiculous. So no, I don't want this, and i certainly don't want an extension that punches a hole in our security (that would result in blocklisting) unless there is extensive checking and it's implemented in such a way that there is no risk for elevation of anything (which is inherent if you waive Xrays on JS content you don't control).
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-26, 17:20

I'm hoping since I only waived Xray vision on the navigator object and then immediately unwaived it after adding the object and function, there shouldn't be much room for elevation risk, aside from whatever's up with the "then" of the Promise.

As far as abuse goes, I'm hoping the authors of the spec realize it's a bad idea to let sites provide their own text and set up a series of common permissions in the future.

For now, I'm thinking of using “6-and-9” quotation marks around the site-provided text, and then replacing those characters with straight quotes if they're included in the message.
Example using the message [store" your information as "cookies]
might result in a doorhanger that says
website.net wants to "store" your information as "cookies"

becomes
website.net wants to “store" your information as "cookies”

but it might be better to just strip quotes entirely and truncate any double-apostrophes to single ones.
Or, I could apply the multiple-permission method to single-permission request and always show the message "website.net wants permission to access or store specific data about you." in the doorhanger with a "Details" button (and "Accept All" & "Reject All" dropdown options), showing the actual request string from the site as a row in the Details dialog. That might be less quick-and-friendly to the user, but it would make potential abuse a little more difficult.

(Edit: I went with the un-crossed-out options, and added a default-off preference to allow single-choice doorhanger to use the site-provided description, stripping quotes and double-apostrophes if enabled just in case.)

If I can't make everything mentioned above cleanly, then I'll probably just strip it down to "accept all", "reject all", and "ignore all" as the only preferences, only show the preference in the extension's options dialog, and skip the whole doorhanger and details dialog entirely. I'm still not sure about a return to the request() function in navigator, since if I return a value immediately instead of a Promise, sites that use await will work, but .then() won't. Unless I create a fake .then() function, maybe. I dunno.
Last edited by RealityRipple on 2021-06-27, 03:38, edited 2 times in total.

New Tobin Paradigm

Re: Modifying Navigator Object

Unread post by New Tobin Paradigm » 2021-06-26, 17:27

You are skating on such thin ice right now. Spec Authors? Oh you mean EU Communists and/or Google Employees? Yeah they don't care. You just need to stop dude. Stop and kill this code.

Just because you CAN do a thing it does NOT MEAN you SHOULD do that thing.

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-26, 17:47

In this case, it's the Vienna University of Economics and Business and the European Center for Digital Rights (noyb).

New Tobin Paradigm

Re: Modifying Navigator Object

Unread post by New Tobin Paradigm » 2021-06-26, 18:23

New Tobin Paradigm wrote:
2021-06-26, 17:27
Communists

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-26, 19:59

Just tried using

Code: Select all

dpc.request = function(consentRequestsList) { return {consent: [], withdraw: [], _object: []}; };
with no mention of async or promises whatsoever in the added navigator "request()" code. Still getting the 'Security wrapper denied access to property "then" on privileged Javascript object' warning when I use "await". I think means that it's not my Promise that has the wrong context, it's a ".then()" function that's generated because the "request()" function is called with "await", using the context of "request()" by default. This would also explain why using ".then()" in content code doesn't trigger the warning. Since request()'s context will always be navigator, and navigator is a chrome object accessible to content, I think that means I can't do anything about the warning. I also surmise it means that my existing code is not leaking chrome context to content script; rather, the chrome is creating and attempting to expose ".then()" to content, and then stopping itself. However, this all relies on the premise that using "await" always generates a ".then()", even if the awaited function returns a Promise; it's just discarded or never called when the Promise's ".then()" replaces it. If that premise is wrong, then something else is happening and there's still a potential security issue.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35402
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Modifying Navigator Object

Unread post by Moonchild » 2021-06-26, 23:34

You don't seem to understand what impact "await" is. if you use "await" you are converting the function to async and the callback to a promise automatically so you'll still have the internal promise code (i.e. the '.then()' function definition) exposed to content.
I think the fact that you get this warning means it is (or rather should be) properly stopped, but that would also mean that the code shouldn't work and the code inside the callback shouldn't be running (i.e. what you pass to await) -- if it still does, regardless of the warning, then I don't think it's secure.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-27, 01:10

That await = generated promise&then is kind of what my last post led me to. I guess what it boils down to is, do the two ".then()" functions interact at any point? The results I'm seeing make me think that the two are not merged, that the async-generated one is blocked, and that the passed Promise's one is funneled through by treating the Promise as an object during return. Whether that successful ".then()"'s context is modified along the way or sticks to Promise's generator's context, though, I'm not sure of, but I think it isn't modified.

It should be easy to see if it exposes chrome by trying to access a chrome-only property from content, right? I tried accessing gBrowser, window, this, self, funct.caller, (new Error()).stack, window.isSecureContextIfOpenerIgnored, and Components.interfaces, using both await and ".then()". They all returned values consistent with content, not chrome.



Here's the workflow as I see it:
flow.png
key.png
key.png (17.88 KiB) Viewed 495 times
Javascript's context is true to the definition's location, so I'm guessing that "Subject Promise" is initialized using content context but has access to chrome context because it's defined in chrome code, but that "Subject Promise's then" is chromeless because it's initialized using the "subject" parameter's context but not defined in chrome, as Subject as passed directly back as a returned object, so the Promise is never resolved within chrome code.

User avatar
Moonchild
Pale Moon guru
Pale Moon guru
Posts: 35402
Joined: 2011-08-28, 17:27
Location: Motala, SE
Contact:

Re: Modifying Navigator Object

Unread post by Moonchild » 2021-06-27, 11:56

Nice theory, but there's 2 snags, one I already indicated:

First, if your schematic is correct, then request() from the content will never have a result because, as you have drawn in yourself, the await-ed response will never pass through the barrier between chrome and content (because you've unwaived). That means that it can never work because content can only send a signal to chrome and never get anything back. So it would be await-ing a response that it will never get. If it still works anyway then there is 2-way access to chrome context.

Second, there is no such thing as a "no-context" ("chromeless") JS object (which is a generic container for anything, including promises -- promises are objects). Every JS object must by definition have a context. then() is a function defined inside the promise prototype and subject to that object's context. The risk here would be that someone could redefine what then() itself does (other than resolve/reject a promise). The question is then where will it be redefined? in content or chrome?

Now it's quite possible that by a stroke of luck our implementation actually remains secure and XRay actually works with the passed-back result because content clones are made of objects, but you do see my apprehension about doing this with content-supplied data that by definition cannot be trusted. It's not at all straightforward how flow and inheritance works here.

Also, you must sanitize and restrain whatever is passed before it's tossed into a UI element (remember the abuse from displaying content-supplied strings in onbeforeunload and http basic auth? yeah...). That would have to go beyond using different quotes or what not.
"Sometimes, the best way to get what you want is to be a good person." -- Louis Rossmann
"Seek wisdom, not knowledge. Knowledge is of the past; wisdom is of the future." -- Native American proverb
"Linux makes everything difficult." -- Lyceus Anubite

User avatar
RealityRipple
Astronaut
Astronaut
Posts: 644
Joined: 2018-05-17, 02:34
Location: Los Berros Canyon, California
Contact:

Re: Modifying Navigator Object

Unread post by RealityRipple » 2021-06-27, 16:36

I didn't indicate anything was no-context. The "then" of the "Subject Promise" gets its context from the "Subject (Window)", as that's what was used to generate the Promise (with "new subject.Promise"). It's just that because it's created as a Promise rather than an async function, the whole bundle, as you said, is just an Object at that point, so there's no ".then()" or "awaited result" to stop at the chrome/content border - thus, the chrome's ".then()" is blocked, and the content's ".then()" is tunneled as part of the Promise object. It's only in the content's call to await or ".then()" that the Promise object gets "unpacked" and the ".then()" gets called.

I'll definitely test changing Promises; since I am calling "new subject.Promise", I can see why that would be dangerous territory. Because the .request() function is constructed during the content-document-global-created event, I figured that no content javascript would have run yet, but the Promise itself is constructed when the function is called, not when it's created. And since everything in JS is by-reference, the "subject" context (including its definition of Promise) may definitely be a weak point. Maybe I can just compare "subject.Promise" with chrome-context's "Promise"? Or create the Promise during the event and use a variable defined in a higher scope instead of a parameter, maybe? If the Promise is built before any site scripts can run, then if content's Promise is overwritten, it shouldn't be effected, right?



Currently, I've hidden the "single-request-shows-directly-in-the-doorhanger" feature behind a preference, the default now being to show a generic "website.net wants permission to access or store specific data about you." message with a "Details" button that opens a dialog with the site-provided text (even if it is just a single row in this case). The site-provided text also shows up in Page Info -> Permissions as additional rows, prefixed with "ADPC: ". And, of course, a big list of all choices is available in the extension's Options dialog. But yeah, while I was writing the quote-stripping code, I was thinking how who-knows how many unicode characters might look like quotes. And if I strip it to all ASCII, it won't be very internationally-friendly. So, yeah, it's got the potential to be a mess, but I'm hoping that displaying it only in ADPC-related dialogs (or labeling it as ADPC in Page Info) will help to prevent that whole onbeforeunload nightmare.

Locked