Skip to content
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.

Sign up for GitHub

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

Fixing prank: no longer override the sender on lower frames #609

Merged
merged 13 commits into from
Dec 2, 2024
Merged

Fixing prank: no longer override the sender on lower frames #609

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c6c4b44
Fixing up prank
msooseth Nov 28, 2024
1dc2e9e
Updating changelog
msooseth Nov 28, 2024
b9891e6
Update src/EVM.hs
msooseth Nov 29, 2024
499910b
Update src/EVM.hs
msooseth Nov 29, 2024
4ad7fbd
Update src/EVM.hs
msooseth Nov 29, 2024
79fa12a
Update src/EVM.hs
msooseth Nov 29, 2024
c960726
Update src/EVM.hs
msooseth Nov 29, 2024
fba5a26
Update src/EVM.hs
msooseth Nov 29, 2024
e7e7c31
Update src/EVM.hs
msooseth Nov 29, 2024
1b1cb64
Merge branch 'main' into fix_prank
msooseth Nov 29, 2024
665ccac
Let's not have trailing whitespace
msooseth Dec 2, 2024
0371caf
Moving resetCaller update to delegateCall
msooseth Dec 2, 2024
6d61bb6
No need for these, it's already been set
msooseth Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Respect --smt-timeout in equivalence checking
- Fixed the handling of returndata with an abstract size during transaction finalization
- Error handling for user-facing cli commands is much improved
- Fixing prank so it doesn't override the sender address on lower call frames

## [0.53.0] - 2024-02-23

Expand Down
52 changes: 25 additions & 27 deletions src/EVM.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ blankState = do
, gas = initialGas
, returndata = mempty
, static = False
, overrideCaller = Nothing
, resetCaller = True
msooseth marked this conversation as resolved.
Show resolved Hide resolved
}

-- | An "external" view of a contract's bytecode, appropriate for
Expand Down Expand Up @@ -145,6 +147,8 @@ makeVm o = do
, gas = o.gas
, returndata = mempty
, static = False
, overrideCaller = Nothing
, resetCaller = True
msooseth marked this conversation as resolved.
Show resolved Hide resolved
}
, env = env
, cache = cache
Expand All @@ -153,8 +157,6 @@ makeVm o = do
, iterations = mempty
, config = RuntimeConfig
{ allowFFI = o.allowFFI
, resetCaller = True
, overrideCaller = Nothing
, baseState = o.baseState
}
, forks = Seq.singleton (ForkState env block cache "")
Expand Down Expand Up @@ -866,17 +868,16 @@ exec1 = do
forceAddr xTo' "unable to determine a call target" $ \xTo ->
case gasTryFrom xGas of
Left _ -> vmError IllegalOverflow
Right gas ->
Right gas -> do
overrideC <- use $ #state % #overrideCaller
delegateCall this gas xTo xTo xValue xInOffset xInSize xOutOffset xOutSize xs $
\callee -> do
let from' = fromMaybe self vm.config.overrideCaller
let from' = fromMaybe self overrideC
zoom #state $ do
assign #callvalue xValue
assign #caller from'
assign #contract callee
do
resetCaller <- use (#config % #resetCaller)
when (resetCaller) $ assign (#config % #overrideCaller) Nothing
assign #overrideCaller Nothing
msooseth marked this conversation as resolved.
Show resolved Hide resolved
touchAccount from'
touchAccount callee
transfer from' callee xValue
Expand All @@ -889,14 +890,13 @@ exec1 = do
forceAddr xTo' "unable to determine a call target" $ \xTo ->
case gasTryFrom xGas of
Left _ -> vmError IllegalOverflow
Right gas ->
Right gas -> do
overrideC <- use $ #state % #overrideCaller
delegateCall this gas xTo self xValue xInOffset xInSize xOutOffset xOutSize xs $ \_ -> do
zoom #state $ do
assign #callvalue xValue
assign #caller $ fromMaybe self vm.config.overrideCaller
do
resetCaller <- use (#config % #resetCaller)
when (resetCaller) $ assign (#config % #overrideCaller) Nothing
assign #caller $ fromMaybe self overrideC
assign #overrideCaller Nothing
msooseth marked this conversation as resolved.
Show resolved Hide resolved
touchAccount self
_ ->
underrun
Expand Down Expand Up @@ -978,17 +978,16 @@ exec1 = do
Just xTo' ->
case gasTryFrom xGas of
Left _ -> vmError IllegalOverflow
Right gas ->
Right gas -> do
overrideC <- use $ #state % #overrideCaller
delegateCall this gas xTo' xTo' (Lit 0) xInOffset xInSize xOutOffset xOutSize xs $
\callee -> do
zoom #state $ do
assign #callvalue (Lit 0)
assign #caller $ fromMaybe self (vm.config.overrideCaller)
assign #caller $ fromMaybe self overrideC
assign #contract callee
assign #static True
do
resetCaller <- use (#config % #resetCaller)
when (resetCaller) $ assign (#config % #overrideCaller) Nothing
assign #overrideCaller Nothing
msooseth marked this conversation as resolved.
Show resolved Hide resolved
touchAccount self
touchAccount callee
_ ->
Expand Down Expand Up @@ -1101,7 +1100,9 @@ callChecks this xGas xContext xTo xValue xInOffset xInSize xOutOffset xOutSize x
accessMemoryRange xOutOffset xOutSize $ do
availableGas <- use (#state % #gas)
let recipientExists = accountExists xContext vm
let from = fromMaybe vm.state.contract vm.config.overrideCaller
let from = fromMaybe vm.state.contract vm.state.overrideCaller
resetCaller <- use $ #state % #resetCaller
when resetCaller $ assign (#state % #overrideCaller) Nothing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably also set resetCaller to False at the same time

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

having this on callChecks feels a bit odd to me, should it maybe live on delegateCall instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... yeah, we could set resetCaller in delegateCall. However, the let from = .. needs to be here of course. Wanna have a go at putting it in delegateCall? Just create a PR on top of this PR :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, good point! I fixed it :) Let me know what you think!

fromBal <- preuse $ #env % #contracts % ix from % #balance
costOfCall fees recipientExists xValue availableGas xGas xTo $ \cost gas' -> do
let checkCallDepth =
Expand Down Expand Up @@ -1145,9 +1146,6 @@ callChecks this xGas xContext xTo xValue xInOffset xInSize xOutOffset xOutSize x
partial $ UnexpectedSymbolicArg pc (getOpName state) "Attempting to transfer eth from a symbolic address that is not present in the state" (wrap [from])
GVar _ -> internalError "Unexpected GVar"




precompiledContract
:: (?op :: Word8, VMOps t)
=> Contract
Expand Down Expand Up @@ -1799,26 +1797,26 @@ cheatActions = Map.fromList
\sig input -> case decodeStaticArgs 0 1 input of
[addr] -> case wordToAddr addr of
Just a -> do
assign (#config % #overrideCaller) (Just a)
doStop
msooseth marked this conversation as resolved.
Show resolved Hide resolved
assign (#state % #overrideCaller) (Just a)
assign (#state % #resetCaller) True
Nothing -> vmError (BadCheatCode "prank(address), could not decode address provided" sig)
_ -> vmError (BadCheatCode "prank(address) parameter decoding failed" sig)

, action "startPrank(address)" $
\sig input -> case decodeStaticArgs 0 1 input of
[addr] -> case wordToAddr addr of
Just a -> do
assign (#config % #overrideCaller) (Just a)
assign (#config % #resetCaller) False
doStop
assign (#state % #overrideCaller) (Just a)
assign (#state % #resetCaller) False
Nothing -> vmError (BadCheatCode "startPrank(address), could not decode address provided" sig)
_ -> vmError (BadCheatCode "startPrank(address) parameter decoding failed" sig)

, action "stopPrank()" $
\_ _ -> do
assign (#config % #overrideCaller) Nothing
assign (#config % #resetCaller) True
doStop
assign (#state % #overrideCaller) Nothing
msooseth marked this conversation as resolved.
Show resolved Hide resolved

, action "createFork(string)" $
\sig input -> case decodeBuf [AbiStringType] input of
Expand Down Expand Up @@ -1993,7 +1991,6 @@ delegateCall this gasGiven xTo xContext xValue xInOffset xInSize xOutOffset xOut
pushTrace (FrameTrace newContext)
next
vm1 <- get

pushTo #frames $ Frame
{ state = vm1.state { stack = xs }
, context = newContext
Expand All @@ -2014,6 +2011,7 @@ delegateCall this gasGiven xTo xContext xValue xInOffset xInSize xOutOffset xOut
assign #memorySize 0
assign #returndata mempty
assign #calldata calldata
assign #overrideCaller Nothing
msooseth marked this conversation as resolved.
Show resolved Hide resolved
continue xTo

-- -- * Contract creation
Expand Down
4 changes: 2 additions & 2 deletions src/EVM/Types.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,8 +665,6 @@ data BaseState
-- | Configuration options that need to be consulted at runtime
data RuntimeConfig = RuntimeConfig
{ allowFFI :: Bool
, overrideCaller :: Maybe (Expr EAddr)
, resetCaller :: Bool
, baseState :: BaseState
}
deriving (Show)
Expand Down Expand Up @@ -728,6 +726,8 @@ data FrameState (t :: VMType) s = FrameState
, gas :: !(Gas t)
, returndata :: Expr Buf
, static :: Bool
, overrideCaller :: Maybe (Expr EAddr)
, resetCaller :: Bool
}
deriving (Generic)

Expand Down
2 changes: 1 addition & 1 deletion test/test.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,7 @@ tests = testGroup "hevm"
Right e <- reachableUserAsserts c (Just $ Sig "f(address,uint256)" [AbiAddressType, AbiUIntType 256])
assertBoolM "The expression is not partial" $ Expr.containsNode isPartial e
,
test "vm.prank underflow" $ do
test "vm.prank-underflow" $ do
Just c <- solcRuntime "C"
[i|
interface Vm {
Expand Down
Loading