LeastAuthority / metamask-plugin-beta

MIT License
1 stars 4 forks source link

Separate getSelectedAddress if MetaMask origin path from getAccounts #5

Open tacticalchihuahua opened 4 years ago

tacticalchihuahua commented 4 years ago

Follow up on this. What passes origin to this method? Can it be manipulated?

Originally posted by @emeryrose in https://github.com/LeastAuthority/metamask-plugin-beta/pull/1

tacticalchihuahua commented 4 years ago

Further investigation of this code path leads me to believe that even if origin was set to MetaMask by a non-metamask caller, this would only tell the caller the currently selected address/account. I'm not sure if this is supposed to be private or not. It seems like it might be? The else code path check to see if the wallet is unlocked before proceeding, but that path returns different information - which may be more privileged.

Either way, I'm still unclear if this could really be an issue. My suggestion either way might be to move the if origin === 'metamask' path of this method call to another method call altogether like getSelectedAccount and ensure that method is only accessible by metamask. This removes some ambiguity around whether or not other callers can manipulate the origin and whether or not it matters if they do.