search

code-423n4 / 2021-06-pooltogether-findings

0 stars 0 forks source link

staticCall to yieldSource.depositToken doesn't provide any security guarantees #14

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

GalloDaSballo

Vulnerability details

Impact

The assumption that a yield source is valid, just because it has the method depositToken is not a security guarantee

Proof of Concept

I could create any random contract, with that function, that is not a guarantee that the contract will behave as intended

Recommended Mitigation Steps

I believe a better solution would be to have a registry, controlled by governance, that accepts the valid yield sources.

A valid registry ensures the the yield sources are properly maintained.

In summary: There is no security difference between having the check and not having the check, because the check can be sidelined without any effort and doesn’t truly provide any guarantee of the contract being valid.

No checks would save you gas

While having a governance registry would guarantee that the yield sources usable are exclusively the community vetted ones.

asselstine commented 3 years ago

It's possible for a malicious developer to fork our code and create a pool with a rugging yield source. That can't really be helped either way.

We decide which pools to display on https://app.pooltogether.com, so we can vet pools already.