search

code-423n4 / 2021-11-nested-findings

1 stars 1 forks source link

nonReentrant modifier causing DOS for releaseTokens #229

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

hack3r-0m

Vulnerability details

https://github.com/code-423n4/2021-11-nested/blob/main/contracts/FeeSplitter.sol#L116-L128

releaseTokens calls will always fail because it calls releaseToken and it is nonReentrant, so it will cause a revert.

Mitigation:

one of the ways is to copy-paste releaseToken logic into releaseTokens.

adrien-supizet commented 2 years ago

That is not how the reentrancy guard works. The mutex is set to true when entering releaseToken, and free when exiting. So calling multiple times the function in a row does not revert. What would revert is if the external call made in releaseToken was calling releaseToken itself.

alcueca commented 2 years ago

Dispute accepted.

Btw, @adrien-supizet, has anyone mentioned that you should have an internal _releaseToken function, and two releaseToken and releaseTokens functions, with the external functions calling the internal one?

It would save gas over having releaseTokens call releaseToken, which is external to public.

adrien-supizet commented 2 years ago

@alcueca

Dispute accepted.

Btw, @adrien-supizet, has anyone mentioned that you should have an internal _releaseToken function, and two releaseToken and releaseTokens functions, with the external functions calling the internal one?

It would save gas over having releaseTokens call releaseToken, which is external to public.

I suggested it here: https://github.com/code-423n4/2021-11-nested-findings/issues/162