From 3ed7f6de48dfaad096336e4ee70e981469291180 Mon Sep 17 00:00:00 2001 From: Jorge Izquierdo Date: Thu, 18 Oct 2018 22:11:00 +0200 Subject: [PATCH 1/3] WIP: AragonApp signed exec --- contracts/apps/AppStorage.sol | 23 +++++++++++++++ contracts/apps/AragonApp.sol | 54 +++++++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/contracts/apps/AppStorage.sol b/contracts/apps/AppStorage.sol index aedb59f7a..61ff63b12 100644 --- a/contracts/apps/AppStorage.sol +++ b/contracts/apps/AppStorage.sol @@ -18,6 +18,9 @@ contract AppStorage { // keccak256("aragonOS.appStorage.pinnedCode"), used by Proxy Pinned bytes32 internal constant PINNED_CODE_POSITION = 0xdee64df20d65e53d7f51cb6ab6d921a0a6a638a91e942e1d8d02df28e31c038e; + bytes32 internal constant VOLATILE_SENDER_POSITION = keccak256("aragonOS.appStorage.volatile.sender"); + bytes32 internal constant USED_NONCE_POSITION_BASE = keccak256("aragonOS.appStorage.usedNonce"); + function kernel() public view returns (IKernel) { return IKernel(KERNEL_POSITION.getStorageAddress()); } @@ -26,6 +29,14 @@ contract AppStorage { return APP_ID_POSITION.getStorageBytes32(); } + function volatileStorageSender() public view returns (address) { + return VOLATILE_SENDER_POSITION.getStorageAddress(); + } + + function usedNonce(address _account, uint256 _nonce) public view returns (bool) { + return usedNoncePosition(_account, _nonce).getStorageBool(); + } + function setKernel(IKernel _kernel) internal { KERNEL_POSITION.setStorageAddress(address(_kernel)); } @@ -33,4 +44,16 @@ contract AppStorage { function setAppId(bytes32 _appId) internal { APP_ID_POSITION.setStorageBytes32(_appId); } + + function setVolatileStorageSender(address _sender) internal { + VOLATILE_SENDER_POSITION.setStorageAddress(_sender); + } + + function setUsedNonce(address _account, uint256 _nonce, bool _used) internal { + return usedNoncePosition(_account, _nonce).setStorageBool(_used); + } + + function usedNoncePosition(address _account, uint256 _nonce) internal returns (bytes32) { + return keccak256(abi.encodePacked(USED_NONCE_POSITION_BASE, _account, _nonce)); + } } diff --git a/contracts/apps/AragonApp.sol b/contracts/apps/AragonApp.sol index b474c3d01..d4b5815a9 100644 --- a/contracts/apps/AragonApp.sol +++ b/contracts/apps/AragonApp.sol @@ -18,17 +18,39 @@ import "../acl/ACLSyntaxSugar.sol"; // that they are automatically usable by subclassing contracts contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, EVMScriptRunner, ACLSyntaxSugar { string private constant ERROR_AUTH_FAILED = "APP_AUTH_FAILED"; + string private constant ERROR_NONCE_REUSE = "APP_NONCE_REUSE"; + string private constant ERROR_INVALID_SIGNATURE = "APP_INVALID_SIGNATURE"; + + address internal constant ZERO_ADDRESS = address(0); modifier auth(bytes32 _role) { - require(canPerform(msg.sender, _role, new uint256[](0)), ERROR_AUTH_FAILED); + require(canPerform(sender(), _role, new uint256[](0)), ERROR_AUTH_FAILED); _; } modifier authP(bytes32 _role, uint256[] _params) { - require(canPerform(msg.sender, _role, _params), ERROR_AUTH_FAILED); + require(canPerform(sender(), _role, _params), ERROR_AUTH_FAILED); _; } + // TODO: support standard? https://eips.ethereum.org/EIPS/eip-1077 + function exec(address signer, bytes calldata, uint256 nonce, bytes signature) public { + require(!usedNonce(signer, nonce), ERROR_NONCE_REUSE); + bytes32 signedHash = executionHash(calldata, nonce); + + require(isValidSignature(signer, signedHash, signature), ERROR_INVALID_SIGNATURE); + + // This won't be too expensive on Constantinople: https://eips.ethereum.org/EIPS/eip-1283 + setVolatileStorageSender(signer); + setUsedNonce(signer, nonce, true); + bool success = address(this).call(calldata); + if (!success){ + // no need to clean up storage as the entire execution frame is reverted + revertForwadingError(); + } + setVolatileStorageSender(ZERO_ADDRESS); + } + /** * @dev Check whether an action can be performed by a sender for a particular role on this app * @param _sender Sender of the call @@ -66,4 +88,32 @@ contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, EVMScriptRunn // Funds recovery via a vault is only available when used with a kernel return kernel().getRecoveryVault(); // if kernel is not set, it will revert } + + function isValidSignature(address signer, bytes32 hash, bytes signature) public pure returns (bool) { + // TODO: Actually check signature. + return true; // YOLO + } + + function executionHash(bytes calldata, uint256 nonce) public pure returns (bytes32) { + return keccak256(abi.encodePacked(keccak256(calldata), nonce)); + } + + function revertForwadingError() internal { + assembly { + let size := returndatasize + let ptr := mload(0x40) + returndatacopy(ptr, 0, size) + revert(ptr, size) + } + } + + function sender() internal view returns (address) { + // Prevents a sub-frame from re-entering into the app while the signer is authenticated + if (msg.sender != address(this)) { + return msg.sender; + } + + address volatileSender = volatileStorageSender(); + return volatileSender != ZERO_ADDRESS ? volatileSender : address(this); + } } From 3d9172041f11e8920c7e4e3a3fd1bf5efebffe3b Mon Sep 17 00:00:00 2001 From: Jorge Izquierdo Date: Thu, 18 Oct 2018 22:11:37 +0200 Subject: [PATCH 2/3] WIP: AragonApp multiexec (commented out as it is currently crashing solc) --- contracts/apps/AragonApp.sol | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contracts/apps/AragonApp.sol b/contracts/apps/AragonApp.sol index d4b5815a9..fded636f6 100644 --- a/contracts/apps/AragonApp.sol +++ b/contracts/apps/AragonApp.sol @@ -3,6 +3,7 @@ */ pragma solidity ^0.4.24; +// pragma experimental ABIEncoderV2; import "./AppStorage.sol"; import "../common/Autopetrified.sol"; @@ -51,6 +52,19 @@ contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, EVMScriptRunn setVolatileStorageSender(ZERO_ADDRESS); } + /* + // TODO: Currently causing a solidity error: InternalCompilerError + function multiexec(address[] signers, bytes[] calldatas, uint256[] nonces, bytes[] signatures) external { + require(signers.length == calldatas.length); + require(signers.length == nonces.length); + require(signers.length == signatures.length); + + for (uint256 i = 0; i < signers.length; i++) { + exec(signers[i], calldatas[i], nonces[i], signatures[i]); + } + } + */ + /** * @dev Check whether an action can be performed by a sender for a particular role on this app * @param _sender Sender of the call From b98c382cb57791090998c18737bea59e941ecdfd Mon Sep 17 00:00:00 2001 From: Jorge Izquierdo Date: Thu, 18 Oct 2018 22:12:07 +0200 Subject: [PATCH 3/3] Fix lint --- contracts/apps/AragonApp.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/apps/AragonApp.sol b/contracts/apps/AragonApp.sol index fded636f6..741bec1f2 100644 --- a/contracts/apps/AragonApp.sol +++ b/contracts/apps/AragonApp.sol @@ -45,7 +45,7 @@ contract AragonApp is AppStorage, Autopetrified, VaultRecoverable, EVMScriptRunn setVolatileStorageSender(signer); setUsedNonce(signer, nonce, true); bool success = address(this).call(calldata); - if (!success){ + if (!success) { // no need to clean up storage as the entire execution frame is reverted revertForwadingError(); }