Merge 1.4.8 to dev#3302
Conversation
Arbitrary length br_tables are limited only by the maximum stack depth of a wasmi executor. The largest possible br_table doing nothing could execute for about 120ms on my machine and only consumed 7210890 in gas which poses a risk of potential abuse.
Co-authored-by: Fraser Hutchison <190532+Fraser999@users.noreply.github.com>
Co-authored-by: George Pisaltu <georgep@casperlabs.io>
Co-authored-by: George Pisaltu <georgep@casperlabs.io>
Each of the tests triggers a panic with an arithmetic overflow in 4 different places. Co-authored-by: George Pisaltu <georgep@casperlabs.io>
Co-authored-by: George Pisaltu <georgep@casperlabs.io>
Co-authored-by: Fraser Hutchison <fraser@casperlabs.io>
mpapierski
requested review from
EdHastingsCasperAssociation,
Fraser999,
dwerner,
fizyk20,
goral09 and
rafal-ch
|
|
||
| ### Security | ||
| * Implement checks before preprocessing Wasm to avoid potential OOM when initializing table section. | ||
| * Implement checks before preprocessing Wasm to avoid references to undeclared functions or globals. |
There was a problem hiding this comment.
Didn't we have something with gas counter as well?
There was a problem hiding this comment.
Forgot about adding this one entry in 1.4.8. Addressed in 77b5503
| "subcall expected to cost more gas due to storing contract" | ||
| ); | ||
| assert!(add_some_gas_from_session_cost.value() > expected_gas); | ||
| assert!(add_some_gas_via_subcall_cost.value() > expected_gas); |
There was a problem hiding this comment.
These changes look like the cost of execution changed - does it mean that some contracts' execution will change now?
There was a problem hiding this comment.
The intention here was to make a wasm cost "at least the expected_gas amount" as this PR disables import of internal 'gas' function. With expected_gas large enough, it should be a precise measurement. Previously, you could precisely add the amount of gas to the counter with the imported gas function.
| const ARG_GAS_AMOUNT: &str = "gas_amount"; | ||
| const ARG_METHOD_NAME: &str = "method_name"; | ||
|
|
||
| fn consume_gas(amount: i32) { |
There was a problem hiding this comment.
Would be nice to have a comment on this method. Are you sure it will consume exactly amount of gas?
Also, why is amount a signed integer, can't it be unsinged?
There was a problem hiding this comment.
Updated in 6169414 to better reflect what it does
mpapierski
force-pushed
the
merge-1.4.8-to-dev
branch
from
b792fc1 to
6169414
Compare
|
bors r+ |
|
Build succeeded: |
4 participants
This PR merges the 1.4.8 bugfix release into dev by cherry-picking relevant fixes from the history of the v1.4.8 release.
A complete history of v1.4.8 release as compared to dev: dev...v1.4.8
Commits cherry-picked in order starting from
Clippy fixes with recent stable(a369e37).(Merge #115, 2022-08-14)(version bump casper-node and assembly script, 2022-08-12)(Merge pull request #114 from casper-network/regression-20220727-part2, 2022-08-12)(Add changelog and docs., 2022-08-12)(Shuffle ensure_* functions in order, 2022-08-12)(Simplify checking algorithm, 2022-08-12)(Fix clippy issues., 2022-08-12)(Add checks to avoid panics in the gas accounting, 2022-08-11)(Add regression tests for gas counter injector., 2022-08-11)(Restrict access to non-declared globals., 2022-08-10)(Regression tests for global set/get access., 2022-08-10)(Merge #113, 2022-08-03)(further updates required for version bumps, 2022-08-03)(Bumping EE, casper-node and assembly script (to keep from erroring on publish)., 2022-08-02)(Merge pull request #110 from casper-network/regression-20220727, 2022-08-02)(Fix backwards compatibility, 2022-08-02)(Apply suggestions from code review, 2022-08-02)(Rewrite for clarity as @goral09 suggested, 2022-08-01)(Ensure internal gas function cannot be imported., 2022-07-29)(Ensure functions max param size., 2022-07-29)(Ensure module doesnt declare more than 256 globals, 2022-07-29)(Limit the size of a br_table to 256 elements., 2022-07-29)(Apply review comments, 2022-07-29)(Apply review comments, 2022-07-28)(Lower the table size to 4k, 2022-07-28)(Fix the issue by imposing a limit for table size., 2022-07-27)(Add regression test to trigger the issue., 2022-07-27)(Clippy fixes with recent stable, 2022-07-27)