Foundry DeFi Stablecoin CodeHawks Audit Contest
Cyfrin
DeFiFoundry
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Is possible to burn tokens without being the **owner** of `DecentralizedStableCoin`

LaScaloneta

Summary

The DecentralizedStableCoin contract, which extends the ERC20Burnable contract, introduces a vulnerability allowing non-owners to burn tokens. While the burn function of the contract is protected by the onlyOwner modifier, the inherited burnFrom function from the ERC20Burnable contract remains unprotected.

Vulnerability Details

The vulnerability arises from the fact that the DecentralizedStableCoin contract is a derivative of the ERC20Burnable contract, which includes the burnFrom function. The burnFrom function permits a user to burn tokens from any account for which they have approval. However, in the DecentralizedStableCoin contract, the intention seems to be to only allow the owner to burn tokens. This discrepancy opens the door for an exploit.

In the provided POC, it is demonstrated that a non-owner can burn tokens from an account for which they have approval.

function testUserShouldntBurn() public {
DecentralizedStableCoin token = new DecentralizedStableCoin();
address user = makeAddr("user");
token.mint(user, 10);
​
vm.startPrank(user);
vm.expectRevert();
// user cant burn tokens
token.burn(10);
// but...
token.approve(user, 10);
// now we can burn tokens! (this should revert but it doesnt)
token.burnFrom(user, 10);
vm.stopPrank();
​
assertEq(token.balanceOf(address(0x05)), 10, "user should have 10 tokens cause cant burn them");
}

Impact

This vulnerability could allow an attacker to burn tokens from any account, given that they have approval. Depending on the application's context and token's usage it can result potential disruption of the token's economics.

Tools Used

Manual revision

Recommendations

The issue can be mitigated by adding an additional modifier on the burnFrom function, ensuring only the owner can execute it. Or perhaps a better solution is to avoid exteding ERC20Burneable and just use ERC20.

diff --git a/src/DecentralizedStableCoin.sol b/src/DecentralizedStableCoin.sol
index d3652a5..90f2a17 100644
--- a/src/DecentralizedStableCoin.sol
+++ b/src/DecentralizedStableCoin.sol
@@ -36,14 +36,14 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
* This is the contract meant to be governed by DSCEngine. This contract is just the ERC20 implementation of our stablecoin system.
*
*/
-contract DecentralizedStableCoin is ERC20Burnable, Ownable {
+contract DecentralizedStableCoin is ERC20, Ownable {
error DecentralizedStableCoin__MustBeMoreThanZero();
error DecentralizedStableCoin__BurnAmountExceedsBalance();
error DecentralizedStableCoin__NotZeroAddress();
constructor() ERC20("DecentralizedStableCoin", "DSC") {}
- function burn(uint256 _amount) public override onlyOwner {
+ function burn(uint256 _amount) external onlyOwner {
uint256 balance = balanceOf(msg.sender);
if (_amount <= 0) {
revert DecentralizedStableCoin__MustBeMoreThanZero();
@@ -51,7 +51,7 @@ contract DecentralizedStableCoin is ERC20Burnable, Ownable {
if (balance < _amount) {
revert DecentralizedStableCoin__BurnAmountExceedsBalance();
}
- super.burn(_amount);
+ _burn(msg.sender, _amount);
}
function mint(address _to, uint256 _amount) external onlyOwner returns (bool) {

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.