Skip to content

Commit

Permalink
Fix associated storage attribution, struct access
Browse files Browse the repository at this point in the history
  • Loading branch information
forshtat committed Jan 20, 2025
1 parent 3d8f316 commit df3ec83
Showing 1 changed file with 61 additions and 23 deletions.
84 changes: 61 additions & 23 deletions packages/validation-manager/src/ERC7562Parser.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BigNumber } from 'ethers'
import { FunctionFragment, hexZeroPad, Interface } from 'ethers/lib/utils'
import { FunctionFragment, hexZeroPad, Interface, keccak256 } from 'ethers/lib/utils'

import {
AddressZero,
Expand All @@ -10,6 +10,7 @@ import {
SenderCreator__factory,
StakeInfo,
StorageMap,
toBytes32,
ValidationErrors
} from '@account-abstraction/utils'

Expand All @@ -18,7 +19,7 @@ import { ERC7562Rule } from './enum/ERC7562Rule'
import { AccountAbstractionEntity } from './AccountAbstractionEntity'
import { bannedOpCodes, opcodesOnlyInStakedEntities } from './ERC7562BannedOpcodes'
import { ValidationResult } from './IValidationManager'
import { AccessedSlots, ERC7562Call } from './ERC7562Call'
import { ERC7562Call } from './ERC7562Call'
import { AltMempoolConfig } from './altmempool/AltMempoolConfig'
import { getOpcodeName } from './enum/EVMOpcodes'

Expand All @@ -29,6 +30,7 @@ export interface ERC7562ValidationResults {
}

export class ERC7562Parser {
private keccack: string[] = []
private ruleViolations: ERC7562Violation[] = []
private currentEntity: AccountAbstractionEntity = AccountAbstractionEntity.none
private currentEntityAddress: string = ''
Expand Down Expand Up @@ -112,7 +114,9 @@ export class ERC7562Parser {
}

private _detectEntityChange (userOp: OperationBase, call: ERC7562Call): void {
if (call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() &&
if (
call.from.toLowerCase() !== AddressZero &&
call.from.toLowerCase() !== this.entryPointAddress.toLowerCase() &&
call.from.toLowerCase() !== this.senderCreatorAddress.toLowerCase()) {
return
}
Expand Down Expand Up @@ -155,6 +159,39 @@ export class ERC7562Parser {
return address
}

/**
* Calculate storage slots associated with each entity.
* keccak( A || ...) is associated with "A"
*
* @param userOp
*/
private _parseEntitySlots (
userOp: OperationBase
): {
[addr: string]: Set<string>
} {
// for each entity (sender, factory, paymaster), hold the valid slot addresses
const entityAddresses = [userOp.sender.toLowerCase(), userOp.paymaster?.toLowerCase(), userOp.factory?.toLowerCase()]
const entitySlots: { [addr: string]: Set<string> } = {}

for (const keccakInput of this.keccack) {
for (const entityAddress of entityAddresses) {
if (entityAddress == null) {
continue
}
const addrPadded = toBytes32(entityAddress)
// valid slot: the slot was generated by keccak(entityAddr || ...)
if (keccakInput.startsWith(addrPadded)) {
if (entitySlots[entityAddress] == null) {
entitySlots[entityAddress] = new Set<string>()
}
entitySlots[entityAddress].add(keccak256(keccakInput))
}
}
}
return entitySlots
}

/**
* Validates the UserOperation and throws an exception in case current mempool configuration rules were violated.
*/
Expand All @@ -180,6 +217,9 @@ export class ERC7562Parser {
throw new Error('Unexpected traceCall result: no calls from entrypoint.')
}

this.keccack = tracerResults.keccak ?? []
this.currentEntity = AccountAbstractionEntity.none
this.currentEntityAddress = ''
this.ruleViolations = []
this.stakeValidationResult = validationResult

Expand Down Expand Up @@ -382,34 +422,32 @@ export class ERC7562Parser {
}
}

checkStorage (userOp: OperationBase, tracerResults: ERC7562Call): void {
Object.entries(tracerResults.accessedSlots).forEach(([address, accessInfo]) => {
this.checkStorageInternal(userOp, address, accessInfo)
})
}

checkStorageInternal (
checkStorage (
userOp: OperationBase,
address: string,
accessInfo: AccessedSlots
tracerResults: ERC7562Call
): void {
if (tracerResults.to.toLowerCase() === this.entryPointAddress.toLowerCase()) {
// Currently inside the EntryPoint deposit code, no access control applies here
return
}
const allSlots: string[] = [
...Object.keys(accessInfo.writes ?? {}),
...Object.keys(accessInfo.reads ?? {}),
...Object.keys(accessInfo.transientWrites ?? {}),
...Object.keys(accessInfo.transientReads ?? {})
...Object.keys(tracerResults.accessedSlots.writes ?? {}),
...Object.keys(tracerResults.accessedSlots.reads ?? {}),
...Object.keys(tracerResults.accessedSlots.transientWrites ?? {}),
...Object.keys(tracerResults.accessedSlots.transientReads ?? {})
]
const entitySlots = {} // TODO: restore
const address = tracerResults.to
const entitySlots = this._parseEntitySlots(userOp)
const addressName = this._tryGetAddressName(userOp, address)
const isEntityStaked = false // TODO
const isFactoryStaked = false // TODO
const isSenderCreation = false // TODO
const isEntityStaked = this._isEntityStaked()
const isFactoryStaked = this._isEntityStaked(AccountAbstractionEntity.factory)
const isSenderCreation = userOp.factory != null
for (const slot of allSlots) {
const isSenderInternalSTO010: boolean = address.toLowerCase() === userOp.sender.toLowerCase()
const isSenderAssociated: boolean = this._associatedWith(slot, userOp.sender.toLowerCase(), entitySlots)
const isEntityInternalSTO031: boolean = address.toLowerCase() === this.currentEntityAddress.toLowerCase()
const isEntityAssociatedSTO032: boolean = this._associatedWith(slot, this.currentEntityAddress, entitySlots)
const isReadOnlyAccessSTO033: boolean = accessInfo.writes?.[slot] == null && accessInfo.transientWrites?.[slot] == null
const isReadOnlyAccessSTO033: boolean = tracerResults.accessedSlots.writes?.[slot] == null && tracerResults.accessedSlots.transientWrites?.[slot] == null

const isAllowedIfEntityStaked = isEntityInternalSTO031 || isEntityAssociatedSTO032 || isReadOnlyAccessSTO033
const isAllowedST031ST032ST033: boolean = isAllowedIfEntityStaked && isEntityStaked
Expand All @@ -426,8 +464,8 @@ export class ERC7562Parser {
) {
description = `unstaked ${this.currentEntity.toString()} accessed ${addressName} slot ${slot}`
} else {
const isWrite = Object.keys(accessInfo.writes ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot)
const isTransient = Object.keys(accessInfo.transientReads ?? {}).includes(slot) || Object.keys(accessInfo.transientWrites ?? {}).includes(slot)
const isWrite = Object.keys(tracerResults.accessedSlots.writes ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot)
const isTransient = Object.keys(tracerResults.accessedSlots.transientReads ?? {}).includes(slot) || Object.keys(tracerResults.accessedSlots.transientWrites ?? {}).includes(slot)
const readWrite = isWrite ? 'write to' : 'read from'
const transientStr = isTransient ? 'transient ' : ''
description = `${this.currentEntity.toString()} has forbidden ${readWrite} ${transientStr}${addressName} slot ${slot}`
Expand Down

0 comments on commit df3ec83

Please sign in to comment.