Note regarding the deprecated target

Below we describe an issue in the zk-bridge repository that was part of the originally contracted scope at the start of the audit. This repository was later deprecated and removed from the scope of the audit after we had found and reported the issue described below. We keep this finding here so that the report accurately reflects the findings we identified and reported during the auditing process. Due to the deprecation of the relevant code, no action is necessary in relation to this finding.

Description

This finding concerns the function checkDynamicFieldLen that is duplicated in

• circuits/fabric/headers/headerutil/utils.go in zk-bridge

• circuits/fabric/smt/utils.go in zk-bridge

This function is used to check that the dynamic length fields of a Ethereum block header have reasonable sizes. It is implemented as follows:

func checkDynamicFieldLen(h types.Header) bool {
	hasErr := 0
	hasErr += checkLen("Difficulty", len(h.Difficulty.Bytes()), 7)
	hasErr += checkLen("Number", len(h.Number.Bytes()), 8)
	hasErr += checkLen("GasLimit", getUintByteLen(h.GasLimit), 4)
	hasErr += checkLen("GasUsed", getUintByteLen(h.GasUsed), 4)
	hasErr += checkLen("Time", getUintByteLen(h.Time), 4)
	hasErr += checkLen("BaseFee", len(h.BaseFee.Bytes()), 7)
	return hasErr == 0
}

While it does check the other dynamic length fields, it is missing a check for Extra. This is a field of type []byte, but it is specified by the Ethereum yellow paper that this field must be at most 32 bytes long. The function should thus also check that this is satisfied.

Impact

Headers with improperly long Extra fields may still make checkDynamicFieldLen return true, signifying proper dynamic field lengths. We are not aware of any caller for which the Extra field could be too long, however.

Recommendations

The checkDynamicFieldLen functions should also ensure that Extra is at most 32 bytes long:

func checkDynamicFieldLen(h types.Header) bool {
	hasErr := 0
	hasErr += checkLen("Difficulty", len(h.Difficulty.Bytes()), 7)
	hasErr += checkLen("Number", len(h.Number.Bytes()), 8)
	hasErr += checkLen("GasLimit", getUintByteLen(h.GasLimit), 4)
	hasErr += checkLen("GasUsed", getUintByteLen(h.GasUsed), 4)
	hasErr += checkLen("Time", getUintByteLen(h.Time), 4)
	hasErr += checkLen("BaseFee", len(h.BaseFee.Bytes()), 7)
+    hasErr += checkLen("Extra", len(h.Extra), 32)
	return hasErr == 0
}

Remediation

The code related to the finding was deprecated before completion of the audit.