ray-lothian / UserAgent-Switcher

A User-Agent spoofer browser extension that is highly configurable
https://webextension.org/listing/useragent-switcher.html
Mozilla Public License 2.0
934 stars 144 forks source link

A custom user-agent can be used to execute arbitrary JavaScript on every page load. #25

Closed mrcnetcraft closed 5 years ago

mrcnetcraft commented 5 years ago

Setting the user-agent to the following string executes that string on every page load:

`;alert(document.domain);`

The user-agent string should be escaped prior to setting it using JavaScript.

ray-lothian commented 5 years ago

Duplicate of https://github.com/ray-lothian/UserAgent-Switcher/issues/19; If a user really wants this then why not! Note that the user is the one who set the UA string.

mrcnetcraft commented 5 years ago

Apologies for the duplicate issue.

While not a security issue, it would be useful if this issue is fixed because at present it prevents the user from setting a user agent containing backticks and template expressions without manually escaping the strings. While I can imagine many users using this extension, and most users using this feature are technically savvy enough to eventually identify what is happening, it is confusing.

Personally I see unexpectedly executing JavaScript, or more likely silently failing to set the user-agent as a misfeature at best. If nothing else, a warning not to use backticks or template expressions would be useful.

ray-lothian commented 5 years ago

@mrcnetcraft please provide example UA strings for testing purpose

mrcnetcraft commented 5 years ago

Setting your user agent to the following runs alert(document.domain) on every page load

 `;alert(document.domain);`

So if you are using Chrome and navigate to example.com, you will be altered with:

example.com says
example.com
[ OK ]

I believe that /extension/common.js#L273 is the culprit:

chrome.tabs.executeScript(tabId, {
  runAt: 'document_start',
  frameId,
  code: `{
    const script = document.createElement('script');
    script.textContent = \`{
      navigator.__defineGetter__('userAgent', () => '${userAgent}');
      navigator.__defineGetter__('appVersion', () => '${appVersion}');
      navigator.__defineGetter__('platform', () => '${platform}');
      navigator.__defineGetter__('vendor', () => '${vendor}');
    }\`;
    document.documentElement.appendChild(script);
    script.remove();
  }`
}, () => chrome.runtime.lastError);

When your user agent is set the the one above, I believe it is substituted as so:

chrome.tabs.executeScript(tabId, {
  runAt: 'document_start',
  frameId,
  code: `{
    const script = document.createElement('script');
    script.textContent = \`{
      navigator.__defineGetter__('userAgent', () => '\`;alert(document.domain);\`');
      navigator.__defineGetter__('appVersion', () => '${appVersion}');
      navigator.__defineGetter__('platform', () => '${platform}');
      navigator.__defineGetter__('vendor', () => '${vendor}');
    }\`;
    document.documentElement.appendChild(script);
    script.remove();
  }`
}, () => chrome.runtime.lastError);

Then the following code is then executed at document_start:

{
  const script = document.createElement('script');
  script.textContent = `{
    navigator.__defineGetter__('userAgent', () => '`;alert(document.domain);`');
    navigator.__defineGetter__('appVersion', () => '${appVersion}');
    navigator.__defineGetter__('platform', () => '${platform}');
    navigator.__defineGetter__('vendor', () => '${vendor}');
  }`;
  document.documentElement.appendChild(script);
  script.remove();
}

Which is equivalent to:

{
  const script = document.createElement('script');
  script.textContent = `{
    navigator.__defineGetter__('userAgent', () => '`;
   alert(document.domain);
  document.documentElement.appendChild(script);
  script.remove();
}

And lastly, in console you see a syntax error for VM:<some number>:

{
  navigator.__defineGetter__('userAgent', () => '

The same issue also occurs with ${appVersion}, ${platform}, and ${vendor} but they are not immediately as obvious to set. It would probably also be a good idea to look elsewhere in the codebase for other uses of user input in template literals to construct javascript, html, or css. And lastly, here are some extra user agents which also break things:

Alert popup:

`;alert(document.domain);`

Logs to console:

`;console.log(document.domain);`

Syntax error in console:

`

Uncaught SyntaxError: missing ) after argument list in console:

'

Another alert popup:

'); alert('1
ray-lothian commented 5 years ago

Thanks. Should have been fixed by https://github.com/ray-lothian/UserAgent-Switcher/commit/8b93a731b0ec8ef6419455924eca2a8df80f284d#diff-d7d96c7982b979aa422398cc4cbf6911R267

mrcnetcraft commented 5 years ago

This fixes

`;alert(document.domain);`

But does not fix

'); alert('1

It also has the disadvantage that you cannot include backticks in your user agent. Lastly, the choice of the NULL character for the replacement is a curious one. I can imagine that there are some systems that will react strangely to it because the NULL character terminates C style strings.

ray-lothian commented 5 years ago

how about https://github.com/ray-lothian/UserAgent-Switcher/commit/31c12610f2918115ba48f1a244b04e9bddd0f8ee#diff-d7d96c7982b979aa422398cc4cbf6911R273?

mrcnetcraft commented 5 years ago

I downloaded the extension from GitHub and tried both of the user agents in my previous comments. Neither worked, and of the top of my head, cannot think of a way to bypass the changes. You can't use `, ", or \ to escape the string, and while ' is left unencoded, by switching from single quotes to double quotes, that attack vector is also blocked. Thanks for fixing this.

ray-lothian commented 5 years ago

Neither worked

You mean it got fixed right? perfect!

mrcnetcraft commented 5 years ago

Correct, apologies for not being clearer.