-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible issue in ERC7579Utils.decodeBatch
#5395
Comments
It is possible to create a user op where It seems that the extent of an exploit would be to cause the account to pay for gas for a user op that should've been rejected in an earlier phase.
The question we should answer is whether the account should protect against malformed |
The issue described above requires that the calldata ERC-7579 includes two function that deal with function execute(bytes32 mode, bytes calldata executionCalldata) external;
function executeFromExecutor(bytes32 mode, bytes calldata executionCalldata) external returns (bytes[] memory returnData); In both cases the In the context of an ERC-4337 operation, we still have to consider the validation ot the user operation. In the validation stage, the calldata will be part of the PackedUserOperation. Offsets in the decoded calldata could point to any (dynamic) part of the user operation beyound the calldata section itself. That includes Technically, a signer would be able to sign a mallformed calldata that would be missinterpreted during validation (if validation does the decoding... in most cases it wont), but that would then fail execution anyway. That would grief gas from the account/paymaster. This griefing is however not different from that caused by any user operation that passes validation and reverts during execution. It could be possible to trigger the early failure during the validation phase. This would require traversing the structure in depth during the decoding, and checking all offsets are not spilling beyond the input
Are there any other context where we expect all the calls to |
Recently, v5.2-rc.1 included a fix to a known issue in
ERC7579Utils.decodeBatch
. The issue was identified as originating from missing checks that solidity automatically inserts when doing adecode
operation. These checks were not present in v5.2-rc.0The way these checks are currently implemented is as follows:
bytes
(in calldata) as an array, we check that thebytes
is long enough so that all the "first-level elements" of the array fit inside thebytes
. Because the array contains dynamic objects, the "first-level elements" are only pointers (offsets) to the actual elements.bytes
object. It is only when dereferencing these "pointers" that solidity checks weither they are "valid".However, solidity does that by checking that the dereferenced object is in the calldata as defined by
calldatasize
. This means that the pointers could target data that is outside thebytes
object while still being in allocated calldata.This could potentially cause issues, though it is not 100% clear how this could be exploited and what the consequences would be. Currently no POC of this bad behavior exists.
Signers of ERC-7579 batch operations (usually within the scope of of an ERC-4337 user operation) should verify that the calldata they sign is properly formed. A possible adversarial senario would exist if the signer is a session key with limited access to the account. This issue, if confirmed, could potentially cause the validation and execution phases to missmatch, allowing the session key to perform restricted operations.
The text was updated successfully, but these errors were encountered: