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

Remove ACL bypass when Kernel is not set from AragonApp #325

Closed
izqui opened this Issue Jun 4, 2018 · 1 comment

Comments

Projects
None yet
2 participants
@izqui
Copy link
Member

izqui commented Jun 4, 2018

https://github.com/aragon/aragonOS/blob/dev/contracts/apps/AragonApp.sol#L32

This issue has been brought up by both audits as a major issue. I think given our knowledge of the system it doesn't seem like a big problem, but IMO there is no need to deploy production code that makes out unit testing of apps easier.

We should remove the address(kernel) == address(0) || ... shortcut from AragonApp and create an AppProxyMock that automatically sets the kernel to a kernel that will allow allow actions.

Pseudocode:

contract KernelAllowAllMock is Kernel {
  function hasPermission(address _who, address _where, bytes32 _what, bytes _how) public view returns (bool) {
     return true;
  }
}

contract AppProxyAllowAllMock is AppProxyBase {
  address base;

  function AppProxyMock(address _base)
           AppProxyBase(new KernelAllowAllMock(), bytes32(0), new bytes(0)) {
    base = _base;
  }
  
  function implementation() public view returns (address) {
    return base;
  }
}

Then when testing apps, they would need to be tested through the mocked proxy

@izqui izqui added the audit: cd label Jun 4, 2018

@izqui

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment