Back to Madalad's contests

zkSync Era System Contracts contest - Madalad's results

Rely on math, not validators.

General Information

Platform: Code4rena

Start Date: 10/03/2023

Pot Size: $180,500 USDC

Total HM: 6

Participants: 19

Period: 9 days

Judge: GalloDaSballo

Total Solo HM: 2

Id: 221

League: ETH

zkSync

Findings Distribution

Researcher Performance

Rank: 17/19

Findings: 1

Award: $237.70

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositoryzkSync

Findings Information

🌟 Selected for report: unforgiven

Also found by: 0xSmartContract, Dravee, HE1M, Madalad, brgltd, bshramin, gjaldon, joestakey, minaminao, rbserver, rvierdiiev, supernova

Labels

bug
grade-b
QA (Quality Assurance)
Q-12

Awards

237.7048 USDC - $237.70

External Links

Link to GitHub issue

Low Risk Summary

IssueInstances
[L-01]Solidity's ecrecover is vulnerable to signature malleability1
[L-02]Downcasting uint or int may result in overflow4

Total issues: 2

Total instances: 5

 

Non-Critical Summary

IssueInstances
[N-01]Ether may become trapped in the contract5
[N-02]Use indexed for event parameters6
[N-03]Use fixed compiler version13
[N-04]Use appropriate function naming convention3
[N-05]Lines too long3
[N-06]Update import usages50
[N-07]Use scientific notation/underscores for large values1
[N-08]Remove/Resolve open TODOs1
[N-09]Remove unused imports1

Total issues: 9

Total instances: 83

 

Low Risk Issues

[L-01] Solidity's ecrecover is vulnerable to signature malleability

The built-in EVM precompile ecrecover is susceptible to signature malleability, which could lead to replay attacks. References: https://swcregistry.io/docs/SWC-117, https://swcregistry.io/docs/SWC-121, and https://medium.com/cryptronics/signature-replay-vulnerabilities-in-smart-contracts-3b6f7596df57

While this is not immediately exploitable, this may become a vulnerability if used elsewhere.

Consider using OpenZeppelin's ECDSA library (which prevents this malleability) instead of the built-in function.

Instances: 1

File: contracts/DefaultAccount.sol

189:         address recoveredAddress = ecrecover(_hash, v, r, s);

 

[L-02] Downcasting uint or int may result in overflow

Consider using OpenZeppelin's SafeCast library to prevent unexpected overflows.

Instances: 15

File: contracts/ContractDeployer.sol

320:             SystemContractHelper.setValueForNextFarCall(uint128(value));

File: contracts/libraries/RLPEncoder.sol

29:                 encoded[0] = bytes1(uint8(hbs + 0x81));

59:                 encoded[0] = bytes1(uint8(_len + _offset));

64:                 encoded[0] = bytes1(uint8(_offset + hbs + 56));

 

 

Non-Critical Issues

[N-01] Ether may become trapped in the contract

Consider rethinking payable functions, or implementing a sweep functionality.

Instances: 5

File: contracts/BytecodeCompressor.sol

35:     function publishCompressedBytecode(

File: contracts/L2EthToken.sol

80:     function withdraw(address _l1Receiver) external payable override {

File: contracts/ContractDeployer.sol

128:     function create2(

144:     function create(

232:     function forceDeployOnAddresses(ForceDeployment[] calldata _deployments) external payable {

 

[N-02] Use indexed for event parameters

Index event fields make the field more quickly accessible to off-chain tools that parse events.

If the variable is value type (uint, address, bool), then using indexed saves gas. Otherwise, each index field costs extra gas during emission.

Instances: 6

File: contracts/interfaces/IL2StandardToken.sol

6:     event BridgeMint(address indexed _account, uint256 _amount);

8:     event BridgeBurn(address indexed _account, uint256 _amount);

File: contracts/interfaces/IEthToken.sol

22:     event Mint(address indexed account, uint256 amount);

24:     event Transfer(address indexed from, address indexed to, uint256 value);

26:     event Withdrawal(address indexed _l2Sender, address indexed _l1Receiver, uint256 _amount);

File: contracts/interfaces/INonceHolder.sol

14:     event ValueSetUnderNonce(address indexed accountAddress, uint256 indexed key, uint256 value);

 

[N-03] Use fixed compiler version

A floating pragma risks a different compiler version being used in production vs testing, which poses security risks.

Instances: 13

File: contracts/EmptyContract.sol

3: pragma solidity ^0.8.0;

File: contracts/ImmutableSimulator.sol

3: pragma solidity ^0.8.0;

File: contracts/L1Messenger.sol

3: pragma solidity ^0.8.0;

File: contracts/MsgValueSimulator.sol

3: pragma solidity ^0.8.0;

File: contracts/BytecodeCompressor.sol

3: pragma solidity ^0.8.0;

File: contracts/AccountCodeStorage.sol

3: pragma solidity ^0.8.0;

File: contracts/L2EthToken.sol

3: pragma solidity ^0.8.0;

File: contracts/SystemContext.sol

3: pragma solidity ^0.8.0;

File: contracts/KnownCodesStorage.sol

3: pragma solidity ^0.8.0;

File: contracts/NonceHolder.sol

3: pragma solidity ^0.8.0;

File: contracts/DefaultAccount.sol

3: pragma solidity ^0.8.0;

File: contracts/ContractDeployer.sol

3: pragma solidity ^0.8.0;

File: contracts/BootloaderUtilities.sol

3: pragma solidity ^0.8.0;

 

[N-04] Use appropriate function naming convention

According to Solidity's style guide and popular convention, function names should be prefixed with an underscore if and only if they are private or internal.

See here for more details.

Instances: 3

File: contracts/BootloaderUtilities.sol

43:     function encodeLegacyTransactionHash(Transaction calldata _transaction) internal view returns (bytes32 txHash) {

138:     function encodeEIP2930TransactionHash(Transaction calldata _transaction) internal view returns (bytes32) {

228:     function encodeEIP1559TransactionHash(Transaction calldata _transaction) internal view returns (bytes32) {

 

[N-05] Lines too long

Solidity's style guide states lines should be kept within 79 characters.

Also, if lines exceed 164 characters then a horizontal scroll bar will be required when viewing the file on Github.

Extensions such as prettier are a simple solution.

Instances: 3

File: contracts/ContractDeployer.sol

7: import {CREATE2_PREFIX, CREATE_PREFIX, NONCE_HOLDER_SYSTEM_CONTRACT, ACCOUNT_CODE_STORAGE_SYSTEM_CONTRACT, FORCE_DEPLOYER, MAX_SYSTEM_CONTRACT_ADDRESS, KNOWN_CODE_STORAGE_CONTRACT, ETH_TOKEN_SYSTEM_CONTRACT, IMMUTABLE_SIMULATOR_SYSTEM_CONTRACT} from "./Constants.sol";

File: contracts/libraries/EfficientCall.sol

24:  * 3. `ptr.pack` - Do the concatenation between the lowest 128 bits of the pointer itself and the highest 128 bits of `_value`. It is typically used to prepare the ABI for external calls.

File: contracts/libraries/TransactionHelper.sol

85:             "Transaction(uint256 txType,uint256 from,uint256 to,uint256 gasLimit,uint256 gasPerPubdataByteLimit,uint256 maxFeePerGas,uint256 maxPriorityFeePerGas,uint256 paymaster,uint256 nonce,uint256 value,bytes data,bytes32[] factoryDeps,bytes paymasterInput)"

 

[N-06] Update import usages

To improve readability and adhere to the rule of modularity and modular programming: only import what you need. Specific imports with curly braces allow us to apply this rule better.

For example:

import {ERC721} from "solmate/tokens/ERC721.sol";

Instances: 50

File: contracts/ImmutableSimulator.sol

5: import "./interfaces/IImmutableSimulator.sol";

File: contracts/L1Messenger.sol

5: import "./interfaces/IL1Messenger.sol";

6: import "./libraries/SystemContractHelper.sol";

7: import "./libraries/EfficientCall.sol";

File: contracts/MsgValueSimulator.sol

5: import "./libraries/EfficientCall.sol";

File: contracts/BytecodeCompressor.sol

5: import "./interfaces/IBytecodeCompressor.sol";

6: import "./Constants.sol";

7: import "./libraries/Utils.sol";

8: import "./libraries/UnsafeBytesCalldata.sol";

File: contracts/AccountCodeStorage.sol

5: import "./interfaces/IAccountCodeStorage.sol";

6: import "./libraries/Utils.sol";

File: contracts/KnownCodesStorage.sol

5: import "./interfaces/IKnownCodesStorage.sol";

6: import "./libraries/Utils.sol";

7: import "./libraries/SystemContractHelper.sol";

File: contracts/NonceHolder.sol

5: import "./interfaces/INonceHolder.sol";

6: import "./interfaces/IContractDeployer.sol";

File: contracts/DefaultAccount.sol

5: import "./interfaces/IAccount.sol";

6: import "./libraries/TransactionHelper.sol";

7: import "./libraries/SystemContractHelper.sol";

8: import "./libraries/EfficientCall.sol";

File: contracts/ContractDeployer.sol

6: import "./interfaces/IContractDeployer.sol";

9: import "./libraries/Utils.sol";

10: import "./libraries/EfficientCall.sol";

File: contracts/BootloaderUtilities.sol

5: import "./interfaces/IBootloaderUtilities.sol";

6: import "./libraries/TransactionHelper.sol";

7: import "./libraries/RLPEncoder.sol";

8: import "./libraries/EfficientCall.sol";

File: contracts/libraries/Utils.sol

4: import "./EfficientCall.sol";

File: contracts/libraries/SystemContractsCaller.sol

6: import "./Utils.sol";

File: contracts/libraries/EfficientCall.sol

5: import "./SystemContractHelper.sol";

6: import "./Utils.sol";

File: contracts/libraries/TransactionHelper.sol

5: import "../openzeppelin/token/ERC20/IERC20.sol";

6: import "../openzeppelin/token/ERC20/utils/SafeERC20.sol";

8: import "../interfaces/IPaymasterFlow.sol";

9: import "../interfaces/IContractDeployer.sol";

11: import "./RLPEncoder.sol";

12: import "./EfficientCall.sol";

File: contracts/interfaces/IBootloaderUtilities.sol

5: import "../libraries/TransactionHelper.sol";

File: contracts/interfaces/IPaymaster.sol

5: import "../libraries/TransactionHelper.sol";

File: contracts/interfaces/IAccount.sol

5: import "../libraries/TransactionHelper.sol";

File: contracts/Constants.sol

5: import "./interfaces/IAccountCodeStorage.sol";

6: import "./interfaces/INonceHolder.sol";

7: import "./interfaces/IContractDeployer.sol";

8: import "./interfaces/IKnownCodesStorage.sol";

9: import "./interfaces/IImmutableSimulator.sol";

10: import "./interfaces/IEthToken.sol";

11: import "./interfaces/IL1Messenger.sol";

12: import "./interfaces/ISystemContext.sol";

13: import "./interfaces/IBytecodeCompressor.sol";

14: import "./BootloaderUtilities.sol";

 

[N-07] Use scientific notation/underscores for large values

e.g. 1e6 or 1_000_000 instead of 1000000.

Instances: 1

File: contracts/SystemContext.sol

40:     uint256 public difficulty = 2500000000000000;

 

[N-08] Remove/Resolve open TODOs

Instances: 1

File: contracts/L2EthToken.sol

25:     // TODO: Remove this variable with the new upgrade.

 

[N-09] Remove unused imports

Instances: 1

File: contracts/libraries/TransactionHelper.sol

9: import "../interfaces/IContractDeployer.sol";

 

#0 - GalloDaSballo

2023-03-31T09:58:55Z

[L-01] Solidity's ecrecover is vulnerable to signature malleability 1 Invalid, checked above - 3

[L-02] Downcasting uint or int may result in overflow 4 Invalid due to lack of proof

[N-01] Ether may become trapped in the contract 5 L

[N-02] Use indexed for event parameters 6 Disputing - 1

[N-03] Use fixed compiler version 13 NC

[N-04] Use appropriate function naming convention 3 NC

[N-05] Lines too long 3 NC

[N-06] Update import usages 50 NC

[N-07] Use scientific notation/underscores for large values 1 R

[N-08] Remove/Resolve open TODOs 1 Ignoring for now

[N-09] Remove unused imports NC

1L 1R 5N -4

#1 - c4-judge

2023-04-06T18:59:52Z

GalloDaSballo marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter