Github username: @goheesheng
Submission hash (on-chain): 0x446cb46b124596ec6673fcbed77a2bd521e243ed528d34555d24b30885cfcb7e
Severity: high
Description:Description\
The token.transferFrom function at the end of investMint assumes that the user has already approved the contract to spend their tokens. If a user is tricked into calling this function without understanding what it does, they could unintentionally transfer their tokens to this contract.
It's also worth noting that the function doesn't perform any checks on the msg.sender other than the onlyEOA modifier. If a malicious actor can trick a user into executing this function, they could potentially cause the user to lose their tokens.
Attack Scenario\
1) The attacker creates a malicious contract that mimics the behavior of a legitimate contract. The malicious contract might have a similar name or functionality to deceive users.
2) The attacker deploys the malicious contract to the blockchain.
The attacker then creates a phishing website or application that looks identical to the legitimate website or application associated with the legitimate contract. The phishing website will prompt users to interact with the malicious contract.
3) The attacker spreads the phishing website or application through various channels, such as social media, email, or messaging platforms, to lure unsuspecting users.
4) When a user visits the phishing website or application and interacts with the malicious contract, the contract's code will use tx.origin to check the user's address.
5) Since tx.origin returns the address that initiated the transaction, not the contract that called the current function, the malicious contract will access the user's address instead of the contract's address.
6) The attacker can then exploit the user's trust by performing unauthorized actions on their behalf, such as transferring funds, accessing sensitive data, or executing malicious code.
function investMint(bool _isDai) external onlyEOA {
SaleState _saleState = saleState;
require(_saleState > SaleState.NOT_ACTIVE, "PRESALE_NOT_ACTIVE");
require(_saleState < SaleState.OVER, "PRESALE_ROUND_FINISHED");
IERC20 token;
token = _isDai ? Dai : Frax;
uint256 tokenAmount;
uint256 typePrice;
uint256 cvgAmount;
if (_saleState == SaleState.PRESEED) {
(tokenAmount, typePrice, cvgAmount) = _setPreseedSupply();
} else {
(tokenAmount, typePrice, cvgAmount) = _setSeedSupply();
}
uint256 _nextTokenId = nextTokenId;
//Update & Associate the tokenId with the presales Info
presaleInfoTokenId[_nextTokenId] = PresaleInfo(TYPE_PRESEED_SEED, cvgAmount);
// Mint
_mint(msg.sender, _nextTokenId);
//Update for next presaler
nextTokenId++;
// Transfer
token.transferFrom(msg.sender, address(this), tokenAmount);
}
https://github.com/hats-finance/Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777/blob/f43c5d9bc6b30c9f488e34836f09dc04d8f7361f/contracts/PresaleVesting/SeedPresaleCvg.sol#L174C5-L205C6
Revised Code File (Optional)
-- Change modifer tomsg.sender instead of tx.origin. \
or\
-- Add a require statement that checks if the contract's allowance of the user's tokens is greater than or equal to tokenAmount. If it's not, the function will revert with the message "NOT_ENOUGH_ALLOWANCE". This ensures that the function will only proceed if the contract has been approved to transfer the necessary amount of tokens.
function investMint(bool _isDai) external onlyEOA {
SaleState _saleState = saleState;
require(_saleState > SaleState.NOT_ACTIVE, "PRESALE_NOT_ACTIVE");
require(_saleState < SaleState.OVER, "PRESALE_ROUND_FINISHED");
IERC20 token;
token = _isDai ? Dai : Frax;
uint256 tokenAmount;
uint256 typePrice;
uint256 cvgAmount;
if (_saleState == SaleState.PRESEED) {
(tokenAmount, typePrice, cvgAmount) = _setPreseedSupply();
} else {
(tokenAmount, typePrice, cvgAmount) = _setSeedSupply();
}
// Check that the contract has enough allowance to perform the transfer
require(token.allowance(msg.sender, address(this)) >= tokenAmount, "NOT_ENOUGH_ALLOWANCE");
uint256 _nextTokenId = nextTokenId;
//Update & Associate the tokenId with the presales Info
presaleInfoTokenId[_nextTokenId] = PresaleInfo(TYPE_PRESEED_SEED, cvgAmount);
// Mint
_mint(msg.sender, _nextTokenId);
//Update for next presaler
nextTokenId++;
// Transfer
token.transferFrom(msg.sender, address(this), tokenAmount);
}
Hello,
Thanks a lot for your attention.
Phishing attacks are not considered as an issue and this contract is Out of scope.
In conclusion we have so to consider this issue as invalid.
Github username: @goheesheng Submission hash (on-chain): 0x446cb46b124596ec6673fcbed77a2bd521e243ed528d34555d24b30885cfcb7e Severity: high
Description: Description\ The token.transferFrom function at the end of investMint assumes that the user has already approved the contract to spend their tokens. If a user is tricked into calling this function without understanding what it does, they could unintentionally transfer their tokens to this contract.
It's also worth noting that the function doesn't perform any checks on the msg.sender other than the onlyEOA modifier. If a malicious actor can trick a user into executing this function, they could potentially cause the user to lose their tokens.
Attack Scenario\ 1) The attacker creates a malicious contract that mimics the behavior of a legitimate contract. The malicious contract might have a similar name or functionality to deceive users. 2) The attacker deploys the malicious contract to the blockchain. The attacker then creates a phishing website or application that looks identical to the legitimate website or application associated with the legitimate contract. The phishing website will prompt users to interact with the malicious contract. 3) The attacker spreads the phishing website or application through various channels, such as social media, email, or messaging platforms, to lure unsuspecting users. 4) When a user visits the phishing website or application and interacts with the malicious contract, the contract's code will use tx.origin to check the user's address. 5) Since tx.origin returns the address that initiated the transaction, not the contract that called the current function, the malicious contract will access the user's address instead of the contract's address. 6) The attacker can then exploit the user's trust by performing unauthorized actions on their behalf, such as transferring funds, accessing sensitive data, or executing malicious code.
Attachments
Proof of Concept (PoC) File\
https://github.com/hats-finance/Convergence-Finance---IBO-0x0e410e7af8e70fc5bffcdbfbdf1673ee7b3d0777/blob/f43c5d9bc6b30c9f488e34836f09dc04d8f7361f/contracts/PresaleVesting/SeedPresaleCvg.sol#L93C1-L96C6
Revised Code File (Optional)
-- Change modifer tomsg.sender instead of tx.origin. \ or\ -- Add a require statement that checks if the contract's allowance of the user's tokens is greater than or equal to tokenAmount. If it's not, the function will revert with the message "NOT_ENOUGH_ALLOWANCE". This ensures that the function will only proceed if the contract has been approved to transfer the necessary amount of tokens.
(Reference) [https://hackernoon.com/hacking-solidity-contracts-using-txorigin-for-authorization-are-vulnerable-to-phishing]