QA Report

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L179
https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L182

Vulnerability details

Impact

ERC20 implementations are not always consistent. Some implementations of transfer and transferFrom could return ‘false’ on failure instead of reverting. It is safer to wrap such calls into require() statements or use safe wrapper functions implementing return value/data checks to handle these failures.

It is observed safeTransfer() is being used in the withdraw function, this should be replicated in the _calcRewardIntegral function

Proof of Concept

Unsafe transfer calls were found in the following locations:

1. https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L179

2. https://github.com/code-423n4/2022-02-concur/blob/main/contracts/ConvexStakingWrapper.sol#L182

Tools Used

1. Manual Analysis

Recommended Mitigation Steps

Check the return value and revert on 0/false or use safeTransfer OpenZeppelin’s SafeERC20 wrapper functions.

[GalloDaSballo]

Dup of #165

[JeeberC4]

Adding to QA Report #81

[GalloDaSballo]

Finding is valid

[GalloDaSballo]

1

[JeeberC4]

Generating as QA Report as warden's actual QA Report was invalidated by judge. Preserving original title: Unhandled return value of transfer in ConvexStakingWrapper