Skip to content

Commit

Permalink
overwrite protection for window-provider
Browse files Browse the repository at this point in the history
and walletRouter

We had to protect our injected window.ethereum provider from being
overwritten, because the load time is not deterministic with the content
scripts.

It was also important not to loose any provider that has been injected
before we get there and to be able to switch back to them, so
implemented a very minimal `window.walletRouter`, which keeps
track of all the providers that we interact with and tried to set
`window.ethereum`.

The protection wasn't feasible to be implemented with Proxies
because a property on the window object had to be protected
and to do so the whole window object should have been replaced
by the proxied object. (At least that's my current understanding.)

Getters and setters can't be defined with a default value setting
so it made sense to create `window.walletRouter`.

Sadly the setter is not run if `Object.defineProprty` is used, and
couldn't find any method to register these actions. Meaning
we can't store those providers that tried to set the object
using it. Eg. rabby wallet.

When injecting the provider into `window.ethereum` multiple content
scripts can set the same object, but the last one to set will be
visible and used by the dApps. (Unless we protect our provider
from being overwritten.)

The load order of the extensions is based on their chromestore id, which
is generated from the public key of the extension. This means that the
content script of other extensions can start runing before us and
after us, and we have no control over this.

The content scripts are started based on the order defined by their ids
but they run in parallel meaing their load and run time also
affects the final order.
  • Loading branch information
Gergo Nagy committed Apr 13, 2022
1 parent 7469384 commit 9c9378c
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 49 deletions.
59 changes: 24 additions & 35 deletions env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,42 +4,31 @@
// result in a handful of type errors. See PR #196.

declare module "styled-jsx/style"

type WalletProvider = {
on: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
removeListener: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
}

type WindowEthereum = WalletProvider & {
isMetaMask?: boolean
isTally?: boolean
autoRefreshOnNetworkChange?: boolean
}
interface Window {
tally?: {
isTally: boolean
on: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
removeListener: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
}
ethereum?: {
isMetaMask?: boolean
isTally?: boolean
on?: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
removeListener?: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
autoRefreshOnNetworkChange?: boolean
walletRouter?: {
currentProvider: WalletProvider
providers: WalletProvider[]
}
oldEthereum?: {
isMetaMask?: boolean
isTally?: boolean
on?: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
removeListener?: (
eventName: string | symbol,
listener: (...args: unknown[]) => void
) => unknown
autoRefreshOnNetworkChange?: boolean
tally?: WalletProvider & {
isTally: boolean
}
ethereum?: WindowEthereum
oldEthereum?: WindowEthereum
}
44 changes: 30 additions & 14 deletions src/window-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,37 @@ import TallyWindowProvider from "@tallyho/window-provider"
// The window object is considered unsafe, because other extensions could have modified them before this script is run.
// For 100% certainty we could create an iframe here, store the references and then destoroy the iframe.
// something like this: https://speakerdeck.com/fransrosen/owasp-appseceu-2018-attacking-modern-web-technologies?slide=95
window.tally = new TallyWindowProvider({
postMessage: (data: WindowRequestEvent) =>
window.postMessage(data, window.location.origin),
addEventListener: (fn: WindowListener) =>
window.addEventListener("message", fn, false),
removeEventListener: (fn: WindowListener) =>
window.removeEventListener("message", fn, false),
origin: window.location.origin,
Object.defineProperty(window, "tally", {
value: new TallyWindowProvider({
postMessage: (data: WindowRequestEvent) =>
window.postMessage(data, window.location.origin),
addEventListener: (fn: WindowListener) =>
window.addEventListener("message", fn, false),
removeEventListener: (fn: WindowListener) =>
window.removeEventListener("message", fn, false),
origin: window.location.origin,
}),
writable: false,
configurable: false,
})

// let's be a bit more pushy but stable: if window.ethereum is occupied let's make a backup
// so we can reset to the original if tally is NOT set to be the default wallet
if (window.ethereum) {
window.oldEthereum = window.ethereum
if (!window.walletRouter) {
Object.defineProperty(window, "walletRouter", {
value: {
currentProvider: window.tally,
providers: [...(window.ethereum ? [window.ethereum] : [])],
},
writable: false,
configurable: false,
})
}

// and set window.ethereum by default, so it's available from the very beginning
window.ethereum = window.tally
Object.defineProperty(window, "ethereum", {
get() {
return window.walletRouter?.currentProvider || window.tally
},
set(newProvider) {
window.walletRouter?.providers.push(newProvider)
},
configurable: false,
})

0 comments on commit 9c9378c

Please sign in to comment.