FINDINGS

Detailed Summary of Findings

Pie Chart

Static Analysis

No major issues were found. Some false positive errors were reported by the tool. Al the other issues have been categorized above according to their level of severity.

Manual Review

High Severity Issues

No High Severity issues were found.

Medium Severity Issues

M-01. Transfer ETH by using transfer may cause this transaction to fail

M-01. Transfer ETH by using transfer may cause this transaction to fail Transferring ETH by using transfer may cause this transaction to fail. Due to the fact that transfer() and send() forward exactly 2,300 gas to the recipient. This hardcoded gas stipend aimed to prevent reentrancy vulnerabilities, but this only makes sense under the assumption that gas costs are constant. Recently EIP 1884 was included in the Istanbul hard fork. One of the changes included in EIP 1884 is an increase in the gas cost of the SLOAD operation, causing a contract's fallback function to cost more than 2300 gas.

Instances:

File: MDUToken.sol:301:

function Investingtoken( ) payable public returns(bool success){
    uint256tokens= msg.value / tokenPrice;
    _balances[_msgSender()] += tokens;
    _balances[_creator] -= tokens;
    recipient.transfer(msg.value); //@audit Use call for transferring eth instead of transfer
    receivedfund += msg.value;
    return true
}

Recommended Mitigation Steps:

Use call{value: msg.value}() instead of transfer() to send Ether.

(bool success, ) = payable(recipient).call{value: msg.value}("");
require (success,"call failed");

Low Severity Issues

L-01. Avoiding the use of floating Pragma.

Contracts should be deployed with the same compiler version and flags that they have been tested with thoroughly. Locking the pragma helps to ensure that contracts do not accidentally get deployed using, for example, an outdated compiler version that might introduce bugs that affect the contract system negatively.

Instances:

File: MDUToken.sol:3:

pragmas solidity^0.8.15;

Recommended Mitigation Steps:

Avoid Floating pragma and use fixed solidity version

L-02. Lack of 2-step transfer of ownership.

Ownable2Step is safer than Ownable for smart contracts because the owner cannot accidentally transfer smart contract ownership to a mistyped address. Rather than directly transferring to the new owner, the transfer only completes when the new owner accepts ownership. Also, If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, breaking all functions with the onlyOwner() modifier.

Instances: File: MDUToken.sol:81:

function transferOwnership(address newOwner) public virtual onlyOwner {
    require(
        newOwner != address(0),
        "Ownable: new owner is the zero address"
    );
    _transferOwnership(newOwner);
}
function _transferOwnership(address newOwner) internal virtual{
    address oldOwner = _owner;
    _owner = newowner;
    emit OwnershipTransferred(oldOwner,newOwner);
}

Recommended Mitigation Steps:

Recommend considering implementing a two step process where the owner nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated E A account is a valid and active account.

L-03. Lack of 0-address check.

Checking addresses against zero-address during initialization or during setting is a security best practice. However, such checks are missing in the constructor of the Presale contract during recipient address initializations.

Instances:

File: MDUToken.sol:288:

constructor(address payable_recipient) {
    admin = _msgSender( );
    recipient = _recipient;
    //@audit No check for Zero-address for recipie
}

Recommended Mitigation Steps:

Recommend adding zero-address checks for all initializations of address state variables.

Informational Issues

1-01. Use the latest version of solidity.

When deploying contracts, you should use the latest released version of Solidity. Apart from exceptional cases, only the latest version receives security fixes. Furthermore, breaking changes, as well as new features, are introduced regularly.

Instances: File: MDUToken.sol:3:

pragmas solidity^0.8.15;

Recommended Mitigation Steps: Use the latest version of solidity i.e. 0.8.19 or 0.8.20.

1-02. Contracts should have full test coverage

While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors wil often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit. Contracts should have 90%+ code coverage.

References:

https://gist.github.com/ (opens in a new tab) CloudEllie/6639dbfd7d1809a3baa28bb2895e1d9#31-contracts-should-have-full-test-coverage

Gas Optimisation Issues

G-01. Using > O costs more gas than=! 0 when used on a uint in a require() statement

O is less efficient than=! 0 for unsigned integers (with proof)=! 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas) Proof: While it may seem that > O is cheaper than !=, this is only true without the optimizer enabled and outside a require statement. If you enable the optimizer at 10k AND you're ni a require statement, this wil save gas. You can see this tweet for more proof: https://twitter.com/gzeon/status/1485428085885640706 (opens in a new tab)

Instances:

MDUToken.sol:219: require(_value> 0 && _balances[_msgSende()] >= _value);
 
MDUToken.sol:219: require(_value> 0 && _balances[_msgSende()] >= _value);

Remediation:

I suggest changing > 0 with != 0. Also, please enable the Optimizer.

G-02. Splitting require() statements that use & saves gas.

Require statements including conditions with the && operator can be broken down in multiple require statements to save gas.

Instances:

MDUToken.sol:219: require(_value> 0 && _balances[_msgSender()] >= _value);
 
MDUToken.sol:219: require(_value> 0 && _balances[_msgSender()] >= _value);

Mitigation:

Breakdown each condition in a separate require statement (though require statements should be replaced with custom errors)

G-03. Custom Errors instead of Revert Strings to save Gas.

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met). Starting from Solidity v0.8.4,there is a convenient and gas-efficient way to explain to users why an operation failed through the use of custom errors.

Until now, you could already use strings to give more information about failures (e.g., revert("Insufficient funds.");),but they are rather expensive, especially when it comes to deploy cost, and it is difficult to use dynamic information in them. Custom errors are defined using the error statement, which can be used inside and outside of contracts (including interfaces and libraries).

Instances:

MDUToken.sol:51: require(owner() == _msgSender(), "Ownable:caller is not the owner")

Remediation:

Isuggest replacing revert strings with custom errors.

G-04. Strict inequalities (>) are more expensive than non- strict ones (>=).

Strict inequalities (>) are more expensive than non-strict ones (>=). This is due to some supplementary checks (ISZERO, 3 gas. I suggest using >= instead of > to avoid some opcodes here:)

Instances:

MDUToken.sol:234:    _value > 0 &&

References: https://code4rena.com/reports2022-04-badger-citadel/#g-31--is-cheaper-than (opens in a new tab)