code-423n4 / 2023-03-zksync-findings

6 stars 1 forks source link

Inconsistency in calculating the gas to pay #67

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/KnownCodesStorage.sol#L97 https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L1Messenger.sol#L37

Vulnerability details

Impact

The calculated gas overhead shows inconsistency.

Proof of Concept

During sending the message to L1, to calculate the gasToPay, length of the message plus some overhead should be multiplied by gasPerPubdataBytes. https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L1Messenger.sol#L36-L39

The overhead length is equal to 64 bytes (32 bytes of encoded offset plus 32 bytes of encoded length). https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/L1Messenger.sol#L29-L33

During sending the bytecode to L1 in KnownCodeStorage.sol, to calculate the gasToPay, the same formula is used, except the overhead which is equal to 100. https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/KnownCodesStorage.sol#L97 https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/Constants.sol#L67

This shows inconsistency in calculating the gas to pay when sending data to L1.

Tools Used

Recommended Mitigation Steps

Better to use the same constant overhead for both cases.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #148

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 1 year ago

Will double check, this may be a Low Severity

c4-judge commented 1 year ago

GalloDaSballo marked the issue as not a duplicate

GalloDaSballo commented 1 year ago

Have double checked this finding, and believe that the warden has found an inconsistency between the gas overhead

The difference is there, but I don't think it justifies a Medium Severity (Leak of Value, Denial of Service)

For this reason I believe Low Severity to be most appropriate - QA Math should be unchanged

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)