-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ERC: Standard URI scheme with metadata, value and byte code #67
Comments
Thanks @Gustav-Simonsson , I added the reference to the main text and modified the title to reflect that! |
Why call the Perhaps |
I just don't want to call the variable |
@alexvandesande lets call it
My reasoning is it only asks as (EVM) bytecode when deploying a contract, in every other scenario it cannot be bytecode as far as I know. |
In the JavaScript API the parameter is called How should the data field be encoded? In base64 like https://en.wikipedia.org/wiki/Data_URI_scheme ? |
Two wei: I think we should use hexidecimal for these URIs if possible. Everywhere else we use hex, and it'll be easier to convert to and back. |
@Smithgift I'm all for the hexadecimal representation. The above proposal uses the ICAP as the address. Would it make sense defining support multiple address formats? ICAP, address (incl. checksummed), etc? |
Have you considered security implications of URIs with byte code? Using an URI with a data field is akin to signing a message written by another party: you should either trust that party, or verify the message yourself. Otherwise this can be used for CSRF-style attacks. E.g. suppose that you want to buy a donut, you get an URI from the merchant, check that value is reasonable and submit it. But this transaction will steal your gold/shares/house, as it might encode an instruction to send your assets to someone else. One way to address this is to segregate different assets into different addresses. If you use an address which only holds ether, nothing can be stolen. But this cannot be enforced by a wallet as a wallet might be unaware of a "special powers" an address might have. E.g perhaps an address doesn't hold any assets, but is associated with your identity, which will give a third party to sign a petition on your behalf. The only way to address this is to transfer funds to a fresh address first, which sounds kinda inefficient (now you need two transactions). Other way to address this is to enable 'data' payments only to known whitelisted addresses which wallet can understand. In principle it might be enough to display 'the meaning' of addresses which are known to the wallet. E.g. user wants to buy a donut, gets URI from a merchant and observes that it wants to do something with a gold contract. Understanding that buying a donut shouldn't in any way involve gold, user will cancel a transaction. Ideally if contract is recognized it should parse data and display details of the call being made. But in this case there is a risk that a keypair is shared between several different wallets, and while wallet A knows that address X is a gold contract, wallet B doesn't, so it will happily sign a gold-stealing transaction thinking it's something new and harmless. |
I dont get what has 'data' with stealing anything from your account. You specify value and gas and it can play with just these on receiving end. |
Data can specify instructions to transfer assets held using smart contracts. value is irrelevant here. Perhaps you should read more on CSRF, confused deputy attack, how Ethereum works etc. It's basically like signing a document prepared by someone else without reading it: it can do arbitrary nasty things. |
We did a bit of this on the Reddit thread but I think the issue @killerstorm is talking about is common to any means of sharing the address of a contract, rather than being particular to sharing that address along with parameters. If you're signing what you think is a contract to sign up for a credit card and it's really a contract to give somebody your house, you're going to have a bad day. Bringing parameters along too potentially makes it slightly easier to exploit, because if the "transfer your house" signature happened to be along a dotted line marked "function transferYourHouse()" that'll give the game away if you have to do some work to specify it, but whatever method we end up with for identifying contracts can't safely rely on everything important having an unambiguous function name. |
@killerstorm wouldn't any implementation display all these details and let them be approved by the user before doing anything? Wallets could decide to reject any URL which contains bytecode. Others could display what it does if it's for a known purpose (e.g. it would be easy to parse if it is for calls on the token interface). |
@axic EIP specification should cover wallet's behavior. You can't just leave it up to wallet implementors to decide. Some of them might be not aware of security concerns. |
I don't think this issue is at all related to the uri scheme or bytecode but it's a general problem to any Ethereum client that works with dapps: it has to, as much as it can, display information about what the contract you are about to execute does. There are many ways of doing those that wel be exploring on the wallet, and I hope the community comes with many others:
All these apply either we are talking about a transaction has been initiated by a js call in a dapp, a link or a qrcode.
|
We probably need to add a way for apps to suggest a sending address with a |
But the whole point was that the "from" field would be set by the client. But I suppose "from" and some custom parameters could be useful
|
The client sets all the fields. They're just hints that can be ignored. |
@alexvandesande You have been warned about security implications. If people will lose their property because of the way these URIs are handled you will be responsible for that. @edmundedgar I think normally contracts are structured in such a way that sending ether to that contract without attached data cannot result in unexpected loss (aside from loss of the sent ether which is expected). Thus it's normally safe to sign a simple send transaction as potential risk is well understood and contained. In other words, only an insane contract will do something destructive upon receiving ether. However, potential loss will be restricted to the value held within such an insane contract, which is normally zero. So yes, adding an ability to attach data has serious security implication. Just because things aren't perfect from security perspectives right now doesn't mean it's OK to create new holes. |
@killerstorm the security implications you are suggesting also apply to the current model where the site builds a transaction in javascript and the client just confirms it. How is it specific to this EIP? |
I agree with @killerstorm that data / bytecode should not be a URI parameter. You'd most likely be patching newly-discovered vulnerabilities on a monthly basis until the end of time...that's a hacker's field day to be able to add executable data to a URI. That's why web2 browsers don't allow it. |
It's not a general executable data, it's data meant to be executed only in the context of an EVM. Any vulnerability there that allow execution outside of that environment is a big issue for the ecosystem as a whole that should be patched, independently of the particular way the transaction was initiated. |
@alexvandesande The difference is that when a site builds a transaction, wallet knows the context. It might associate a particular account with a particular context. It can work similarly to the same-origin policy on the web: a web site can only work with its own data. E.g. code which is executed in the context of gmail.com is able to send emails in my name, but code is executed in the context of bash.org cannot. If one uses URIs, the origin/context is not known to the wallet, so it doesn't know what account will be safe to use. It might ask the user about the context, e.g. "Did you receive this URI from a trusted source?", but users are generally bad with that kind of stuff. |
@killerstorm in that case, clients could implement variations of same origin policies. Mist will know where the link was clicked. If someone builds an ethereum extension, it would know the page origin too. If someone is building an app that accepts any external links, then it's up to that app to handle their policies. And in all those cases it's essential for ethereum clients to show as much information as possible about the transaction as they can. A malicious app in Mist can also attempt to execute a transaction that will not do what it says it will do and we can't prevent it (if an app wants to receive payment in digix gold, it's acceptable for them to request a transaction from the digix contract, even thou they are not digix themselves). |
I really like the function parameter. It's an elegant solution. I like others think it's a bad idea to even standardize a huge security hole like the bytecode parameter. The way tech journalists work once fraudsters figure out how to use it, it will unfortunately be a setback for Ethereum. |
@killerstorm would replacing pure data by the function parameters (recently added) address your concerns? |
If the bytecode field is a security risk, how are function parameters not? It surely helps a lot to show what the method name is without knowing the contract (as you cannot retrieve that from the method hash), but it wont reassure you that Wouldn't it make more sense having a security recommendation what wallets should and shouldn't do? I do like having both |
@johnda98 please have a look at ERC-681 |
This proposal has been superseded by #681 |
Thanks Ligi and Alex :-) |
@Arachnid You nominated this as an "EIPs that should be merged". Can you please share your notes on that here? |
@alexvandesande could you please help me find an example of eip 681 that includes sending arbitrary transactions? I find the spec to be very unclear. |
I wonder if we should introduce an |
…mplate-bugs fix broken link to CAIP-104 in readme.md
This proposal is inspired by BIP 21 and could apply to IBAN address format but can be extended to other proposed addresses formats. Imagine these scenarios:
In all these scenarios, the provider wants to set up internally a transaction, with a recipient, an associated number of ethers (or none) and optional byte code, all without requiring any fuss from the end user that is expected simply to choose a sender and authorise the transaction.
Currently implementations for this are wonky: shape shift creates tons of temporary addresses and uses an internal system to check which one correspond to which metadata, there isn't any standard way for stores that want payment in ether to put specific metadata about price on the call and any app implementing contracts will have to use different solutions depending on the client they are targeting.
I propose adding, beyond address, also optional byte code and value to any proposed address standard. Of course this would make the link longer, but it should not be something visible to the user, instead it should be shown as a visual code (QR or otherwise), a link or some other way to pass the information.
If properly implemented in all wallets, this should make execution of contracts directly from wallets much simpler as the wallet client only needs to put the byte code by reading the qr code.
If we follow the bitcoin standard, the result would be:
Other data could be added, but ideally the client should take them from elsewhere in the blockchain, so instead of having a
label
or amessage
to be displayed to the users, these should be read from an identity system or metadata on the transaction itself.Example:
Clicking this link would open a transaction that would try to send 5 unicorns to address deadbeef. The user would then simply to approve, based on each wallet UI.
Without byte code
Alternatively, the byte code could be generated by the client and the request would be in plain text:
Example:
This is the same function as above, to send 5 unicorns from he sender to deadbeef, but now with a more readable function, which the client converts to byte code.
The text was updated successfully, but these errors were encountered: