Summary
The burnProfile function in SoulboundProfileNFT performs an external interaction (_burn) before state modifications, which doesn't follow the Checks-Effects-Interactions pattern.
Vulnerability Details
In SoulboundProfileNFT.sol, the burnProfile function:
Calls _burn (external interaction) first
Then modifies state variables with delete operations
Does not follow the CEI pattern
function burnProfile() external {
uint256 tokenId = profileToToken[msg.sender];
require(tokenId != 0, "No profile found");
require(ownerOf(tokenId) == msg.sender, "Not profile owner");
_burn(tokenId);
delete profileToToken[msg.sender];
delete _profiles[tokenId];
emit ProfileBurned(msg.sender, tokenId);
}
Impact
Pattern Violation : Does not follow the Checks-Effects-Interactions pattern
Future Risk : Could be vulnerable if _burn is modified to include callbacks
Maintainability : Makes the code more prone to vulnerabilities during updates
Severity: LOW - Currently safe as _burn doesn't include callbacks, but could be risky in future updates
Tools Used
Manual code review
Best practices analysis
Recommendations
Reorder operations to follow CEI pattern:
function burnProfile() external {
uint256 tokenId = profileToToken[msg.sender];
require(tokenId != 0, "No profile found");
require(ownerOf(tokenId) == msg.sender, "Not profile owner");
delete profileToToken[msg.sender];
delete _profiles[tokenId];
_burn(tokenId);
emit ProfileBurned(msg.sender, tokenId);
}
Add reentrancy protection:
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract SoulboundProfileNFT is ERC721, ReentrancyGuard {
function burnProfile() external nonReentrant {
}
}
Consider implementing a two-step burn process:
mapping(address => bool) public burnPending;
uint256 public constant BURN_DELAY = 1 days;
mapping(address => uint256) public burnRequestTime;
function requestBurn() external {
uint256 tokenId = profileToToken[msg.sender];
require(tokenId != 0, "No profile found");
require(ownerOf(tokenId) == msg.sender, "Not profile owner");
burnPending[msg.sender] = true;
burnRequestTime[msg.sender] = block.timestamp;
emit BurnRequested(msg.sender, tokenId);
}
function executeBurn() external {
require(burnPending[msg.sender], "No burn requested");
require(block.timestamp >= burnRequestTime[msg.sender] + BURN_DELAY, "Burn delay not elapsed");
uint256 tokenId = profileToToken[msg.sender];
delete burnPending[msg.sender];
delete burnRequestTime[msg.sender];
delete profileToToken[msg.sender];
delete _profiles[tokenId];
_burn(tokenId);
emit ProfileBurned(msg.sender, tokenId);
}
