`Governable` configuration can be backrun

[code423n4]

Handle

Ruhum

Vulnerability details

Impact

The abstract Governable contract is used throughout the contract to enforce access control to sensitive functions. The contract's configuration functions can be back-run allowing the user to bypass the access controls:

• onlySoulUpdateProposal()
• onlySuccessfulProposal()
• governanceApproved()

Depending on the contracts they get access to there are multiple attack vectors. Here are some I've found:

• Flan:
  • can whitelist themselves as minter and freely print Flan tokens
• FlanBackstop:
  • can configure a backer
  • if the FlanBackstop contract was already approved by the Flan contract as a minter, the attacker is able to trigger purchasePyroFlan with the backer they configured themselves. They can trade worthless tokens for PyroFlan directly, and drain the Uniswap Pair containing the worthless tokens and the Flan.
  • If FlanBackstop was not approved as a minter there's no attack vector. The devs simply have to redeploy the contract.
• Limbo:
  • configure the crossing configuration to let the migration send the tokens to them instead of the AMM
  • adjust/configure souls
  • this one is unlikely tho since at the point of the attack there probably won't be any funds in this contract.

Generally, they have access to everything that should be protected by Governable.

Proof of Concept

Governable.setDAO(): https://github.com/code-423n4/2022-01-behodler/blob/main/contracts/DAO/Governable.sol#L68-L72

The function sets the DAO which will be called by the access control modifiers. If the attacker sets DAO to be a custom contract with the same interface they can bypass all the requests to it. The function only allows the DAO to be set if DAO == address(0) || msg.sender == DAO || !configured.

The first part will always be false since all of the above contracts call setDAO() in their constructor. We can expect the passed dao parameter to always be != address(0).

• https://github.com/code-423n4/2022-01-behodler/blob/main/contracts/Flan.sol#L17
• https://github.com/code-423n4/2022-01-behodler/blob/main/contracts/FlanBackstop.sol#L33

The second part is then also always false since the attacker won't control the DAO contract.

The third part is the issue. The Governable contract has a state variable configured. It is used to check whether the configuration is finished or not. Through the endConfiguration() function it can be set to true. This function is callable by anyone.

So right after a contract inheriting Governable is deployed, the attacker back-runs the deployment by calling setDAO() and endConfiguration() within the same transaction. After that, the setDAO() function is not callable by anyone but the attacker.

The note above the endConfiguration() function suggests that a contract will be configured at some point after deployment but not right after: https://github.com/code-423n4/2022-01-behodler/blob/main/contracts/DAO/Governable.sol#L21-L24

So maybe the attacker doesn't even have to back-run the deployment.

All of the access control modifiers depend on the DAO contract to specify whether the access should be given or not: https://github.com/code-423n4/2022-01-behodler/blob/main/contracts/DAO/Governable.sol#L29-L61 Controlling the DAO contract means we can bypass all of that.

Worst case scenario would be if the contracts are being used without configured being true.

Tools Used

none

Recommended Mitigation Steps

Add access controls to endConfiguration(). Don't allow setDAO() to be called if it's not configured yet. Instead only the DAO should be able to do that.

[gititGoro]

Assuming I don't make it clear which wallet is the deployer, is this attack a serious threat? Is it feasible for a bot to scan all contract creation events for the right byte code within the time frame? In this scenario, we'd have to have bots that a) know about limbo b) know the precise deployed bytecode to snoop for c) monitor all contact creation events or somehow know the deployer wallet d) care about limbo

If so, I could deliberately obfuscate the deployed bytecode slightly since it's likely to be a string compare. So adding in an unused var or something should be enough.

Beyond this, let's say I deploy all the contracts and someone silently sneaks themselves in as a whitelist. After configuration I observe all events involving each contract and notice a suspicious whitelisting. I then discard the contracts and start again.

Is there some generic way that attackers can monitor and strike? I know that front running bots don't need to understand the code they're attacking. They simply look for a units of TokenA goes into contractX and b units of token B come out. They front run if b/a is better than the market rate. I don't think a similar thing occurs here.

[jack-the-pug]

Downgrade to Low. This is very similar to initializer can be frontrun. Technically possible but not very practical as an attack vector. It's an issue worth noticing and the recommended mitigation is not too bad.