Strange network bug on the extension page

Thanks for your suggestion to use content script in the iframe, that’s interesting. Though it will require rewriting half of my code, so I need to think about it.

It is happened this way historically that I had 2 separate execution engines in this project. The first one was initially manipulating the iframe directly, which was fine when using test framework on the same domain where the tested web app lives as a tool, and then it evolved into the current one (1 of 2). I don’t really want to rewrite it from scratch, because it works fine in Chrome (the current remade version of it).

I still wonder what is the problem with my XHR requests… Don’t you know the reason?

Well, if you do have "<all_urls>" permission, than you should be able to fetch almost anything. The exceptions will be those protected hosts, like other extensions pages or mozilla pages or internal about:* pages.

Are you sure it’s the fetch that fails? Any chance you could paste it here?

Also, try these in your console in your addon, they should work:

await (await fetch('https://group-speed-dial.fastaddons.com/')).text()
await (await fetch('http://www.virustotal.com/')).text()
await (await fetch('https://group-speed-dial.fastaddons.com/')).text()

SyntaxError: missing ) after argument list

I guess something is wrong with your code…

Yes, I can paste the code here, but how can it help you to debug? I think ZIP would be more useful, though for me the installation my own of extensions from ZIPs does not work for some unknown reason…

var test_instances = []

function startTest(url, test_index) {
	var req = new XMLHttpRequest()
	req.open("GET", url, true)
	req.responseType = "document"
	req.onload = function() {
		processResponse(req.response, url)
		test_instances.push({ index: test_index, url: url, pos: 0, result: null, ac: 0, cc: 0, pc: 0, xhrc: 0, errors: 0, paused: false })
		document.getElementById('ifr').test = test_instances[test_instances.length-1]
		var test = tests[entries[test_index].test_id].data
		runOperation(test, 0, test_instances.length-1)
	}
	req.send(null)
}

function sendRequest(url, method, data) {
	var ti = document.getElementById('ifr').test
	var test = tests[ti.index]
	var test_index = ti.index
	var pos = ti.pos
	var method = method.toUpperCase()
	if (!url.match(/^http/)) {
		url	= url.length ? ti.url.split('/').slice(0, -1).join('/') + '/' + url : ti.url
	}
	if (!data) data = null
	var req = new XMLHttpRequest()
	req.open(method, url, true)
	req.responseType = "document"
	if (method == "POST") {
		req.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded')
	}
	req.onload = function() {
		processResponse(req.response, url)
		if (pos < tests[test_index].data.length-1) {
			ti.paused = false
			runOperation( tests[test_index].data, pos+1, test_index)
		}
	}
	req.send(data)
}

function processResponse(response, url) {
	result = response && response.children[0]
	var ifr = document.getElementById('ifr')
	while (ifr.contentDocument.children.length) {
		ifr.contentDocument.removeChild(ifr.contentDocument.children[0])
	}
	var uri = new URL(url)
	result.innerHTML = result.innerHTML.replace(/(href|src)="\//g, '$1="' + uri.origin + '/')
	       .replace(/(href|src|action)="(?!http:|https:|android-app:)([a-z0-9_-]+)/g, '$1="' + uri.origin + uri.pathname + '/$2')
		   .replace(/(fetch\(["'])\//g, '$1' + uri.origin + '/')
		   .replace(/(fetch\(["'])(?!http:|https:|android-app:)([a-z0-9_-]+)/g, '$1' + uri.origin + uri.pathname + '/$2')
		   .replace(/(\.open\(['"](?:GET|POST)['"],\s*['"])\//g, '$1' + uri.origin + '/')
		   .replace(/(\.open\(['"](?:GET|POST)['"],\s*['"])(?!http:|https:|android-app:)([a-z0-9_-]+)/g, '$1' + uri.origin + uri.pathname + '/$2')
		   .replace(/url\(([^("]+)\)/g, 'url("$1")')
		   .replace(/(href|src)=([^" \t]+)(\s+)/g, '$1="$2"$3')
		   .replace(/(url\("?)\//g, '$1' + uri.origin + '/')
		   .replace(/(url\("?)(?!http:|https:|data:)([a-z0-9_-]+)/g, '$1' + uri.origin + uri.pathname + '/$2')
	ifr.contentDocument.appendChild(result)
	ifr.contentWindow.eval("window.alerts = []; window.confirms = []; window.promts = []; window.xhr = []; " +
	                       "window.store = {}; var prevent_xhr = false; " +
	                       "var confirm_answer = true; var promt_answer = 'lightning'\n" +
	                       "window.alert = function(msg) { console.log(msg); this.alerts.push(msg) }\n" +
	                       "window.confirm = function(msg) { this.confirms.push(msg); return this.confirm_answer }\n" +
	                       "window.promt = function(msg) { this.promts.push(msg); return this.promt_answer }\n" +
	                       "if (!window.XMLHttpRequest.prototype._open) { window.XMLHttpRequest.prototype._open = window.XMLHttpRequest.prototype.open; window.XMLHttpRequest.prototype.open = function(method, url, async) { window.xhr.push({ method: method, url: url, data: null }); if (!window.prevent_xhr) this._open(method, url, async); }}\n" +
	                       "if (!window.XMLHttpRequest.prototype._send) { window.XMLHttpRequest.prototype._send = window.XMLHttpRequest.prototype.send; window.XMLHttpRequest.prototype.send = function(data) { window.xhr[window.xhr.length-1].data = data; if (!window.prevent_xhr) this._send(data); }}\n" +
	                       "window.parent = null; window.locationUrl = '" + url + "'\n" +
	                       "if (!window._fetch) { window._fetch = window.fetch; window.fetch = function(url, options) { window.xhr.push({ method: 'GET', url: url, data: null }); if (options.method) window.xhr[window.xhr.length-1].method = options.method; if (options.body) window.xhr[window.xhr.length-1].data = options.body; if (!window.prevent_xhr) return window._fetch(url, options); }}\n" +
	                       "window.parent = null; window.locationUrl = '" + url + "'")
	var s = result.getElementsByTagName('script')
	for (var i = 0; i < s.length; i++) {
		try { ifr.contentWindow.eval(s[i].innerText) } catch(ex) { console.error(ex) }
	}
	fireEvent(ifr.contentWindow, 'DOMContentLoaded')
	fireEvent(ifr.contentWindow, 'load')
	if (ifr.test) ifr.test.paused = false
}

What really fails here is this:

	var req = new XMLHttpRequest()
	req.open(method, url, true)
	req.responseType = "document"
	if (method == "POST") {
		req.setRequestHeader('Content-Type', 'application/x-www-form-urlencoded')
	}
	req.onload = function() {
		processResponse(req.response, url)
		if (pos < tests[test_index].data.length-1) {
			ti.paused = false
			runOperation(tests[test_index].data, pos+1, test_index)
		}
	}
	req.send(data)

It looks like the email notification with the preview message replaces apostrophes with (whatever that is :smiley:).
Try to copy it from here, not email.

Also, when I said to paste the code here, I meant just the specific fetch that fails with a specific URL. You’ve mentioned it won’t work with “http” protocol.

The problem is not apostrophes, I think, I copied it from here, the single quotes were okay.

You’ve mentioned it won’t work with “http” protocol.

No, on my extension page neither of the protocols work.

What Firefox version are you running? You must be able to execute it without any SyntaxError issues.

I am running Firefox 62.

Tried in Firefox 77 portable (or whatever is the latest version), it does not complain about the syntax, but still fails:

Content Security Policy: The page’s settings blocked the loading of a resource at https://group-speed-dial.fastaddons.com/ (“default-src”).

I tried to run it from the settings page and from the regular https site, it does not matter. And it works perfectly in latest Chrome (!)

Btw, the problem with my extension is also present on the latest version of Firefox (I checked it first before posting here), so it is not a fixed bug - I guess I’m not really wasting your time for nothing with this

So first of all just a friendly reminder :slight_smile: that Firefox 62 is not supported my Mozilla anymore and doesn’t contains many security patches. So you really should upgrade to at least Firefox ESR 68.

The CSP error you see you should not see if you execute it from your addon page if you have the "<all_urls>" permission in the manifest. I’ve tested it, so I’m 100% sure :slight_smile:.

Now some bad news. From what I can see in your code, you are using or violating two policies:

  • “Unsafe assignment to innerHTML”
  • “eval can be harmful.”

Both of these are kind of forbidden and your addon may not pass the review.

I know why innerHTML is bad (performance impact and sometimes security). But what is really funny is the thing that it complains about any innerHTML assignment, not just for <script> tags (that I have learned to avoid using assigning to textContent). It is impossible to write readable maintainable code without it completely, yes, I use it a lot in the internals. Thank God it passed the review despite of these warnings.

What for eval - it is completely impossible to make things working without it in this particular case. Btw, I don’t accept anything from the user there.

Firefox 62 is not supported my Mozilla anymore and doesn’t contains many security patches

I had issues with focus on contenteditable DIVs on a very important site (local russian social network) in its IM module from version 63 (also in 64 and 65), that’s why I stopped updating. Maybe now it is already fixed, but I don’t care :smile:

The CSP error you see you should not see if you execute it from your addon page if you have the "<all_urls>" permission in the manifest. I’ve tested it, so I’m 100% sure

How could this be? I have the permission, and it is not working for me.

screenshot

Maybe I should post my manifest here?

Yes, please do.

Know that both, “eval” and “innerHTML” are a security risk and for the reviewer can be really hard to tell if you are using “safely” or if you actually have “good intentions”.
Also both can be replaced with safe alternatives, almost every time!
In this case your eval is basically just executing javascript in the iframe, right? You have content scripts for that. And “innerHTML” is just DOM manipulation.

Yeah, I know I have content scripts. The thing is that I want a single code base for “web version” (shich of course needs setting CORS headers on the remote) and the extension. And of course content scripts are available only in the extensions API, not in the basic web app.

I could replace eval with injecting script tags, I guess, but I see absolutely no difference here in terms of security.

DOM manipulation can be too heavy in terms of code when you need to create large nested structures of elements, such as table rows with cells inside them and links and icons inside that cells, for example, no? :slight_smile:

Here is the manifest contents:

{
  "manifest_version": 2,
  "name": "Lightning",
  "version": "1.0.0j",

  "icons": {
      "128": "icons/icon128.png",
      "16": "icons/icon16.png",
      "48": "icons/icon48.png"
  },
  "content_scripts": [
    {
      "matches": [ "<all_urls>" ],
	  "css": [ "styles/baloon.css" ],
      "js": [ "includes/script.js" ]
    }
  ],
  "options_ui": {
      "open_in_tab": true,
      "page": "options.html"
  },
  "background": {
    "scripts": ["js/background.js"]
  },
  "permissions": [
    "storage",
    "tabs"
  ],
  "web_accessible_resources": [
    "<all_urls>",
	"icons/*"
  ],
  "content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",
  "browser_action": {
      "default_title": "Lightning",
      "default_icon": "icons/icon.png",
      "default_popup": "popup.html"
  }

}

So as you can see, you are indeed missing "<all_urls>" permission in the "permissions" list :slight_smile:.

So yes, injecting script tags is not better if you again concatenating code :slight_smile:.

And yes, building DOM is not easy, but doable once you create some helper functions. If you need complex DOM, you should use some library for that, like Vue or React.

Why use heavy and slow libraries like Vue or React if this can be easily done using innerHTML (and in fact it is not much slower than building DOM “by hand” with createElement and appendChild)? HTML parsers built into modern browsers are good and optimized enough to perform this not so difficult task like parsing an HTML markup fragment. Why use Vue/React templates and parse them with Vue/React (it is slower anyway because this parsing is made with JS, not with C/C++)? It’s a really strange solution.

Also I brought up in memory that I am using eval in order to let users of my extensions use eval on their pages in test scripts: it can be extremely useful to evaluate some random expression rather than only get elements’ content and attributes :slight_smile:

So as you can see, you are indeed missing "<all_urls>" permission in the "permissions" list

Sorry, I don’t get it quite right, this manifest was taken from Chrome version. Is Firefox syntax different? Should I right the following:

{
  "manifest_version": 2,
  "name": "Lightning",
  "version": "1.0.0j",
  ...
  "permissions": [
    "storage",
    "tabs",
    "<all_urls>"
  ],
  ...
}

?

Because I already have this line in “web_accessible_resources” list. Is it doing nothing in Firefox?

I’ve mentioned Vue and React for when you need complex DOM. Because doing complicated things with vanilla javascript is never easy and frameworks like this makes it almost a piece of cake. And the performance impact is often negligible for the end user (even if it would take additional 100ms to load the page).

Anyhow, I’m just trying to help you avoid awkward situation when you get email from reviewer saying that you have 10 days to remove your innerHTML and eval :slight_smile:. It happened to me a long time ago with some other forbidden features I was not aware of.

Regarding eval for users - there is a special API for Firefox for this that should help: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/userScripts

And regarding the permissions - it’s the same in Firefox and Chrome, so I don’t really understand how it could ever work in Chrome without that permission. Something is fishy here…

The “web_accessible_resources” is for when you want to allow the target page use your own resources (like icons), see the docs: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/web_accessible_resources

(even if it would take additional 100ms to load the page

Sometimes it is much, much worse. See this article, for example: https://timkadlec.com/remembers/2020-04-21-the-cost-of-javascript-frameworks/

Ah, I got it finally. In Chrome it works the same way: I had to add this permission to use my local icons in my local CSS file for (otherwise no images were loaded and shown). But they were not shown on external pages (where I added something using content scripts). So I think I had no idea why I needed to write this and just did it automatically, so thanks for clarification.

It finally works, thanks in advance for your help! :smiley:

I tried to explore the link you have given about user scripts, but I don’t find it convenient and appropriate here :frowning:

First of all, its support starts with Firefox 68, whereas my approach can work anywhere since Firefox 45 or so (from the first version that implemented the WebExtensions API).

Second, I see that there is a lot of “fine tuning” options available (i.e. when exactly to run the script and so on), but for my purpose of “execute a short line of code and return the result immediately” it is not suitable at all, like shooting birds with cannon or something like that :smiley:

Thirdly, I still don’t understand at which point using this new API is better than injecting <script> in the contentScript code (if the code is coming from the user directly then security is the same, the “one-way” nature of execution without the possibility to pass data simply to the extension is also the same, but the difficulty of implementation is much greater in the userScript scenario). Well, maybe in the second case I can use extension APIs like sendMessage to pass the data to the background page, but they did not show the selective providing of Extension APIs to the userScript in the example…

Finally, I told you I have 2 different execution engines: the second one (“fullscreen” or “tabbed”) does not use eval, instead it injects <script> tags into target page, and in that script a special custom function call is attached which populates the hidden <div> with the data the user wants to receive. This works synchronously and quite well; no eval 's are used there. But the first engine (which is needed for use case outside the extension realm, i.e. in web version), really needs to use eval, because there is no other option available there. And as I already said, I would like to keep a single codebase for the project :wink:

UPD: Now I realize that the idea of data exchange with injecting <script> tags and reading back from a special hidden element can work equally good in web version, too. I will think about rewriting maybe. The only problem is that it makes the code larger and more complicated. But now, thanks to our discussion, I will have a “plan B” solution if they require me to delete eval 's completely from code.

1 Like

I guess the critical line (without which everything breaks in Chrome) is this one:

"content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self'",

Or maybe I am wrong and this only breaks my eval 's. Need to check once more.

Ah yes, I didn’t noticed that. That’s also very special feature that’s in the gray zone :slight_smile:.

Note from the MDN:

  1. Note: Valid examples display the correct use of keys in CSP. However, extensions with ‘unsafe-eval’, ‘unsafe-inline’, remote script, blob, or remote sources in their CSP are not allowed for extensions listed on addons.mozilla.org due to major security issues.

See more info here: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/content_security_policy