Summary
The MartenitsaEvent contract allows users to become producers during the special event by redeeming their 1 HealthToken.
But the MartenitsaEvent contract inherits the MartenitsaToken contract instead of using the instance of the dedicated MartenitsaToken contract which results into MartenitsaEvent contract having its own producers information that will be isolated from the actual MartenitsaToken contract and as a result of which user is actually not a producer even though they participate in the event and will not be able to participate in the marketplace.
The MartenitsaToken is a dedicated contract which stores all the information regarding producers and allows them to produce token and list it on marketplace, and marketplace contract allows only this martenitsa token contract to allow the listing, and as MartenitsaEvent contract has their own token, which will have no relation with that dedicated token contract.
Vulnerability Details
The vulnerability is present in the MartenitsaEvent contract where it inherits the MartenitsaToken contract and as a result of which all the producers related information and producers making Martenitsa will be associated with the MartenitsaEvent contract.
The MartenitsaToken contract is a dedicated token contract for Martenitsa and Marketplace contract utilizes the dedicated MartenitsaToken contract for facilitating the Martenitsa transfers.
But, MartenitsaEvent by inheriting MartenitsaToken acts as a Token contract and all the producers being added by participating in the event and making Martenitsa only happens inside of MartenitsaEvent contract and is thus independent of the dedicated MartenitsaToken contract.
function joinEvent() external {
require(block.timestamp < eventEndTime, "Event has ended");
require(!_participants[msg.sender], "You have already joined the event");
require(!isProducer[msg.sender], "Producers are not allowed to participate");
require(_healthToken.balanceOf(msg.sender) >= healthTokenRequirement, "Insufficient HealthToken balance");
_participants[msg.sender] = true;
participants.push(msg.sender);
emit ParticipantJoined(msg.sender);
(bool success) = _healthToken.transferFrom(msg.sender, address(this), healthTokenRequirement);
require(success, "The transfer is not successful");
@> _addProducer(msg.sender);
}
Here, isProducer is a mapping which is a separate variable of MartenitsaEvent contract which it is inherited by inheriting the MartenitsaToken contract and is actually not the variable associated with MartenitsaToken contract.
Therefore, all the users participating in the event are thus producers inside the MartenitsaEvent contract and not in actual MartenitsaToken contract and thus they are not actually producers as they are not associated with the dedicated MartenitsaToken contract.
function _addProducer(address _producer) internal {
@> isProducer[_producer] = true;
@> producers.push(_producer);
}
Impact
The users become producers on MartenitsaEvent contract and not in the actual MartenitsaToken contract.
The Martenitsa they produce is of no significance as it is in no way related to the actual MartenitsaToken contract and thus it can't be listed on MartenitsaMarketplace as it allows listings only from the actual MartenitsaToken contract.
PoC
Add the test in the file: test/MartenitsaEvent.t.sol
Run the test:
forge test --mt test_UsersParticipatingInEvents_DoesNotActuallyBecomeProducers_AndCantList
function test_UsersParticipatingInEvents_DoesNotActuallyBecomeProducers_AndCantList() public {
address kit = makeAddr("kit");
uint256 healthTokenAmount = martenitsaEvent.healthTokenRequirement();
deal(address(healthToken), kit, healthTokenAmount);
martenitsaEvent.startEvent(1 days);
vm.startPrank(kit);
healthToken.approve(address(martenitsaEvent), healthTokenAmount);
martenitsaEvent.joinEvent();
vm.stopPrank();
assert(martenitsaEvent.isProducer(kit) == true);
assert(martenitsaToken.isProducer(kit) == false);
}
Tools Used
Manual Review, Foundry Unit Test
Recommendations
Instead of inheriting the MartenitsaToken inside MartenitsaEvent contract, use the instance of the dedicated MartenitsaToken contract.
Perform changes as below:
@@ -3,8 +3,9 @@ pragma solidity ^0.8.21;
import {HealthToken} from "./HealthToken.sol";
import {MartenitsaToken} from "./MartenitsaToken.sol";
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
-contract MartenitsaEvent is MartenitsaToken {
+contract MartenitsaEvent is Ownable {
HealthToken private _healthToken;
@@ -13,13 +14,14 @@ contract MartenitsaEvent is MartenitsaToken {
uint256 public eventEndTime;
uint256 public healthTokenRequirement = 10 ** 18;
address[] public participants;
+ MartenitsaToken public martenitsaToken;
mapping(address => bool) private _participants;
event EventStarted(uint256 indexed startTime, uint256 indexed eventEndTime);
event ParticipantJoined(address indexed participant);
- constructor(address healthToken) onlyOwner {
+ constructor(address healthToken) Ownable(msg.sender) {
_healthToken = HealthToken(healthToken);
}
@@ -34,6 +36,10 @@ contract MartenitsaEvent is MartenitsaToken {
emit EventStarted(eventStartTime, eventEndTime);
}
+ function setMartenitsaToken(address _mt) external onlyOwner {
+ martenitsaToken = MartenitsaToken(_mt);
+ }
+
/**
* @notice Function to join to event. Each participant is a producer during the event.
* @notice The event should be active and the caller should not be joined already.
@@ -42,7 +48,7 @@ contract MartenitsaEvent is MartenitsaToken {
function joinEvent() external {
require(block.timestamp < eventEndTime, "Event has ended");
require(!_participants[msg.sender], "You have already joined the event");
- require(!isProducer[msg.sender], "Producers are not allowed to participate");
+ require(!martenitsaToken.isProducer(msg.sender), "Producers are not allowed to participate");
require(_healthToken.balanceOf(msg.sender) >= healthTokenRequirement, "Insufficient HealthToken balance");
_participants[msg.sender] = true;
@@ -60,7 +66,7 @@ contract MartenitsaEvent is MartenitsaToken {
function stopEvent() external onlyOwner {
require(block.timestamp >= eventEndTime, "Event is not ended");
for (uint256 i = 0; i < participants.length; i++) {
- isProducer[participants[i]] = false;
+ _participants[participants[i]] = false;
}
}
@@ -71,12 +77,16 @@ contract MartenitsaEvent is MartenitsaToken {
return _participants[participant];
}
+ function isProducer(address _producer) external view returns (bool) {
+ return _participants[_producer];
+ }
+
/**
* @notice Function to add a new producer.
* @param _producer The address of the new producer.
*/
function _addProducer(address _producer) internal {
- isProducer[_producer] = true;
- producers.push(_producer);
+ _participants[_producer] = true;
+ participants.push(_producer);
}
}
@@ -3,6 +3,7 @@ pragma solidity ^0.8.21;
import "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
+import "./MartenitsaEvent.sol";
contract MartenitsaToken is ERC721, Ownable {
@@ -10,8 +11,9 @@ contract MartenitsaToken is ERC721, Ownable {
address[] public producers;
mapping(address => uint256) public countMartenitsaTokensOwner;
- mapping(address => bool) public isProducer;
+ mapping(address => bool) public _isProducer;
mapping(uint256 => string) public tokenDesigns;
+ MartenitsaEvent public martenitsaEvent;
event Created(address indexed owner, uint256 indexed tokenId, string indexed design);
@@ -23,7 +25,7 @@ contract MartenitsaToken is ERC721, Ownable {
*/
function setProducers(address[] memory _producersList) public onlyOwner{
for (uint256 i = 0; i < _producersList.length; i++) {
- isProducer[_producersList[i]] = true;
+ _isProducer[_producersList[i]] = true;
producers.push(_producersList[i]);
}
}
@@ -33,7 +35,7 @@ contract MartenitsaToken is ERC721, Ownable {
* @param design The type (bracelet, necklace, Pizho and Penda and other) of martenitsa.
*/
function createMartenitsa(string memory design) external {
- require(isProducer[msg.sender], "You are not a producer!");
+ require(isProducer(msg.sender), "You are not a producer!");
require(bytes(design).length > 0, "Design cannot be empty");
uint256 tokenId = _nextTokenId++;
@@ -45,6 +47,14 @@ contract MartenitsaToken is ERC721, Ownable {
_safeMint(msg.sender, tokenId);
}
+ function setMartenitsaEvent(address _martenitsaEvent) public onlyOwner {
+ martenitsaEvent = MartenitsaEvent(_martenitsaEvent);
+ }
+
+ function isProducer(address _producer) public view returns (bool) {
+ return _isProducer[_producer] || martenitsaEvent.getParticipant(_producer);
+ }
+
/**
* @notice Function to get the design of a martenitsa.
* @param tokenId The Id of the MartenitsaToken.
