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.

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

Dynamic buffers #491

Closed
wants to merge 14 commits into from
Closed

Dynamic buffers #491

wants to merge 14 commits into from

Conversation

MrChico
Copy link
Member

@MrChico MrChico commented Aug 27, 2020

Introduces support for dynamic data types in memory and calldata (and all over the place, really) through SMT lists. These are currently only supported by z3.
Some clean up needs to happen before this is ready to merge, but I am at the point where I want to trigger some more tests.

There will also be a need for some more execution modes, such as --no-gas-checks or --no-memory-checks in order to avoid constantly queries z3 about stuff that isn't particularly interesting from a security perspective.

vm1 calldata' callvalue' caller'
(contract''
& set EVM.storage store
& set EVM.origStorage store)
Copy link
Contributor

Choose a reason for hiding this comment

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

this was a bug in the previous implementation right?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I think so

Copy link
Contributor

Choose a reason for hiding this comment

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

What tests would catch this?

Copy link
Member Author

@MrChico MrChico Sep 2, 2020

Choose a reason for hiding this comment

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

Maybe some comparison of gas usage between seth --run-tx and seth send

@@ -437,7 +437,7 @@ genAbiValue = \case
AbiTupleType ts ->
AbiTuple <$> mapM genAbiValue ts
where
genUInt n = AbiUInt n <$> arbitraryIntegralWithMax n
genUInt n = AbiUInt n <$> arbitraryIntegralWithMax (2^n-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the fix for the property based tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I think I need to rebase this

pre preVM = let SymbolicBuffer bs = ditch 4 (fst $ view (state . calldata) preVM)
(x, y) = splitAt 32 bs
pre preVM = let StaticSymBuffer bs = ditch 4 $ view (state . calldata) preVM
(x, y) = trace ("calldata length: " <> show (length bs)) $ splitAt 32 bs
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to leave this trace in?

Copy link
Member Author

Choose a reason for hiding this comment

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

nope, it's removed later

@@ -170,7 +170,7 @@ data Cache = Cache
-- | A way to specify an initial VM state
data VMOpts = VMOpts
{ vmoptContract :: Contract
, vmoptCalldata :: (Buffer, (SWord 32)) -- maximum size of uint32 as per eip 1985
, vmoptCalldata :: Buffer -- maximum size of uint32 as per eip 1985
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this comment

Copy link
Member Author

Choose a reason for hiding this comment

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

We assume the length of calldata is limited to a uint32. But this isn't really relied upon here since we don't carry the length around anymore, so I'll remove this comment

-- | Burn gas, failing if insufficient gas is available
burn :: Word -> EVM () -> EVM ()
Copy link
Contributor

Choose a reason for hiding this comment

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

total nit but would put the space back here...

-- RIPEMD-160
0x3 -> num $ (((len input + 31) `div` 32) * 120) + 600
0x3 -> num $ (((l input + 31) `div` 32) * 120) + 600
where l i = fromMaybe (error "unsupported: dynamic data to SHA256") (unliteral $ len input)
Copy link
Contributor

Choose a reason for hiding this comment

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

error message seems wrong here?

-- IDENTITY
0x4 -> num $ (((len input + 31) `div` 32) * 3) + 15
0x4 -> num $ (((l input + 31) `div` 32) * 3) + 15
where l i = fromMaybe (error "unsupported: dynamic data to SHA256") (unliteral $ len input)
Copy link
Contributor

Choose a reason for hiding this comment

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

error message seems wrong here?

@@ -2561,11 +2574,13 @@ costOfPrecompile (FeeSchedule {..}) precompileAddr input =
-- ECMUL
0x7 -> g_ecmul
-- ECPAIRING
0x8 -> num $ ((len input) `div` 192) * (num g_pairing_point) + (num g_pairing_base)
0x8 -> num $ ((l input) `div` 192) * (num g_pairing_point) + (num g_pairing_base)
where l i = fromMaybe (error "unsupported: dynamic data to SHA256") (unliteral $ len input)
Copy link
Contributor

Choose a reason for hiding this comment

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

error message seems wrong here?


symAbiArg (AbiBytesType n)
| n <= 32 = sbytes32
| otherwise = error "bad type"

-- TODO: is this encoding correct?
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TODO still valid?

Copy link
Member Author

Choose a reason for hiding this comment

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

It hasn't really seen any testing yet so I'm not sure

@@ -713,22 +713,18 @@ symvmFromCommand :: Command Options.Unwrapped -> Query EVM.VM
symvmFromCommand cmd = do
caller' <- maybe (SAddr <$> freshVar_) (return . litAddr) (caller cmd)
callvalue' <- maybe (sw256 <$> freshVar_) (return . w256lit) (value cmd)
(calldata', cdlen, pathCond) <- case (calldata cmd, sig cmd) of
-- fully abstract calldata (up to 1024 bytes)
calldata' <- case (calldata cmd, sig cmd) of
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm reading this right, there is currently no command line flag that allows the user to execute with calldata set to a DynamicSymBuffer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right now that's true. You can with loadSymVM though, which I've been experimenting with. Will include it in the cli once things mature

@d-xo
Copy link
Contributor

d-xo commented Sep 2, 2020

There are some test cases in the smt checker test suite that use dynamic data (under OpCalldatacopy in the unsupported opcodes section`). It would be nice to enable these.

Unfortunately the smt checker tests are executed with cvc4 only, as z3 has performance issues when symbolicly executing through the function dispatch code with static symbolic data, so there will probably need to be some rework of the test machinery to allow execution of at least some tests with z3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants