aurora-is-near / aurora-engine

⚙️ Aurora Engine implements an Ethereum Virtual Machine (EVM) on the NEAR Protocol.
https://doc.aurora.dev/develop/compat/evm
326 stars 78 forks source link

proposal: add `assert_one_yocto()` for `call()` #480

Open think-in-universe opened 2 years ago

think-in-universe commented 2 years ago

I'm building one demo of swap wNEAR to USDT on Trisolaris on Aurora with your NEAR account, and noticed one potential security risk if a good number of applications tries to use the call() method on Aurora Engine. (Checkout the demo here: https://github.com/think-in-universe/aurora-defi-on-near (live version: https://think-in-universe.github.io/aurora-defi-on-near/))

Because with the call() method on Aurora Engine, your NEAR account is able to control the assets of the virtual aurora account mapped from your NEAR account, this gives the call() method really high privilege even with a function call key.

For example, if someone is using the demo I have built here, and created a function call for aurora contract, and if the app is evil, and steals the created function call from local storage, then the attacker is able to move the assets from the user's virtual aurora account to anywhere.

So if the practice of calling Aurora contract with the NEAR account is adopt widely, the risk will be quite high. One possible solution is to add assert_one_yocto() in call() to make sure only full access key can call the function. However, I'm not sure whether this will impact the current usage of call() in other scenarios.

mfornet commented 2 years ago

What you have stated is correct! This is designed explicitly this way, if your implicit account id on aurora has some tokens (or other special permissions) it is not expected to allow arbitrary contract calls to aurora.call method.

One possible solution is to add assert_one_yocto() in call() to make sure only full access key can call the function.

Attaching one_yocto_near won't necessarily solve this problem. The contract itself may attach one yocto near. And more importantly, it is possible that some behaviours requires attaching more than one yocto near! Also if you allow arbitrary calls to this function, in the logic of your code it will be required for you to attach one yocto NEAR which defeats the whole idea of attaching it in the first place.


If you want to allow more restrictive arbitrary cross contract calls from your account, one potential solution would be to only call a proxy contract that will run a delegate call. msg.sender won't be your implicit account id when it hits the real target contract.

I won't expect allowing arbitrary calls from a NEAR account to become a common pattern.

think-in-universe commented 2 years ago

Attaching one_yocto_near won't necessarily solve this problem. The contract itself may attach one yocto near. And more importantly, it is possible that some behaviours requires attaching more than one yocto near! Also if you allow arbitrary calls to this function, in the logic of your code it will be required for you to attach one yocto NEAR which defeats the whole idea of attaching it in the first place.

that's right. maybe we can add assert_one_yocto_or_more() to ensure at least one yocto is attached, but you can still attach more. how do you think?

I won't expect allowing arbitrary calls from a NEAR account to become a common pattern.

I see. what's the typical scenarios when call() is used today?

mfornet commented 2 years ago

I see. what's the typical scenarios when call() is used today?

I don't think it is being used much yet. But in general as in NEAR, you would use it to call a particular contract with a particular payload that don't put at risk your balance.

marco-sundsk commented 2 years ago

Attaching one_yocto_near won't necessarily solve this problem. The contract itself may attach one yocto near. And more importantly, it is possible that some behaviours requires attaching more than one yocto near! Also if you allow arbitrary calls to this function, in the logic of your code it will be required for you to attach one yocto NEAR which defeats the whole idea of attaching it in the first place.

There is a definite requirement to have a safe and express way for transferring assets from implicit account back to its near corresponding near account. That would allow near native users to safely and implicitly enjoy aurora contracts service.

The think-in-universe solution (with one or more yocto near deposit) may enhance the security but may sacrifice some user experience (from my side, I won't expect jumping to near wallet when the action is only to transfer assets among my own accounts). Could we figure out an interface that can be called using access key but only allow token transfer to the specific near account id, that is, the one that implicit account generated from.