Summary

The vulnerability arises in a _burnDsc(), line 157 and line 287 from the assumption that the transferFrom method of the ERC20 token always returns a boolean, which is not the case for all ERC20 tokens.

Vulnerability Details

The vulnerability specifically lies in this line of your code:

bool success = i_dsc.transferFrom(dscFrom, address(this), amountDscToBurn);

The transferFrom method is expected to return a boolean. However, this is not always the case for all ERC20 tokens. For Solidity versions >= 0.4.22, contracts will revert on interaction with methods that do not have a return value. Therefore, if the transferFrom method does not return a boolean value, the transaction will fail.

Impact

The impact of this vulnerability could be high. If the code interacts with an ERC20 contract that does not return a boolean from its transferFrom method, the entire transaction will fail. This could potentially lead to lost gas fees and interruption of the function that calls _burnDsc.

Tools Used

manual code review and understanding of ERC20 token standards.

Recommendations

To fix the vulnerability, you need to ensure that transferFrom will not cause the transaction to revert if it doesn't return a boolean. You could do it with OpenZeppelin's SafeERC20
library, which provides safeTransferFrom that handles lack of return value.

Here's an example of how you can use it:

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

// ...

using SafeERC20 for IERC20;

// ...

function _burnDsc(uint256 amountDscToBurn, address onBehalfOf, address dscFrom) private {
s_DSCMinted[onBehalfOf] -= amountDscToBurn;
i_dsc.safeTransferFrom(dscFrom, address(this), amountDscToBurn);
i_dsc.burn(amountDscToBurn);
}

The safeTransferFrom method is designed to handle tokens that return no boolean value from their transfer function, providing an extra level of safety. Thus, it's recommendable to use this function when you're handling with token transfers in Solidity.