makerdao / dss-auto-line

GNU Affero General Public License v3.0
7 stars 14 forks source link

Switch to a Continuous Model #6

Open hexonaut opened 3 years ago

hexonaut commented 3 years ago

There is a potential denial of service attack that can be done by anyone with Dai debt that is larger than the gap. The attack works as follows:

// Max out the debt ceiling (assume the current debt is exactly line - gap, but it works the same no matter what amount)
vat.frob(ilk, ..., gap, ...);

Wait until the ttl expires then execute the following on the next block with high gas:

// Pay back gap amount of debt minus some dust to trigger an increase
vat.frob(ilk, ..., -(gap - dust), ...);

// Call dss-auto-line exec
// Debt ceiling will be set to what it was previously + some dust to trigger a DC raise ttl
autoline.exec(ilk);

// Max out the debt ceiling and repeat
vat.frob(ilk, ..., gap, ...);

I think to solve this we need to remove the discontinuities and switch to a continuous function that can be called any time. This will give us a more accurate measure of what the average Dai debt is rather then sampling at one particular moment which is subject to gaming.

In particular, I propose we use the function line = EMA + gap where EMA is an exponentially weighted moving average of the Dai debt. Most of the logic is the same except we drop the ttl and replace it with a decay factor. We can use the EMA decay factor to control for OSM security. I think the model for both is fairly similar, but we eliminate the discontinuity and thus the ability to game the system.

gbalabasquer commented 3 years ago

We are aware of this issue but I believe there is a bit of overreaction thinking on this problem as something that can easily occur.

A whale can be doing this attack each hour yes, it's possible. But it is more possible than any other genuine DAI minter can front run this person. IMO the possibility that this module (as it is) works in general as intended is a higher win than a remote possibility of this type of attack.

I believe governance should go from simpler to more complex, start with a module that can be easily tested and improved over the future. More complex models are also more gas expensive, so we also need to take that into account.

Worst case, governance can remove/replace it in 48 hours.

brianmcmichael commented 3 years ago

We've added flashloan protection against this vulnerability here which should mitigate some of this concern.

We'd want to keep ttl at some amount equal or greater than the OSM update period to prevent an attacker from exploiting an oracle mispricing. A continuous function would allow an attacker to repeatedly increase the line and max out the adapter before an oracle can be updated or frozen.

hexonaut commented 3 years ago

@gbalabasquer Yes this is a fair point about it being a little odd to maintain this attack especially since it can be remedied with a governance vote a few days later. So maybe we can keep this as a possible defence if this comes up, but proceed with the existing contract.

@brianmcmichael you are correct if we were not weighing the average over time which is why I proposed using an exponentially decaying moving average so if there is an immediate burst of minting it takes time for the moving average to catch up.