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

Fix possible overflow in decode_raw #1383

Merged

Conversation

ClementWalter
Copy link
Member

@ClementWalter ClementWalter commented Sep 3, 2024

Time spent on this PR: 0.2

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

In rlp.decode_raw, the assert_nn(remaining_data_len); doeesn't check that remaining_data_len has not overflown.

Resolves #1376

What is the new behavior?

Each of the decoded values are asserted in RANGE_CHECK, as well as their sum.

A test has been added, failing first and passing after fix.


This change is Reviewable

@ClementWalter ClementWalter merged commit 5f11829 into kkrt-labs:main Sep 4, 2024
6 checks passed
@ClementWalter ClementWalter deleted the cw/fix-overflow-rlp-decode branch September 4, 2024 09:45
matthieuauger pushed a commit to matthieuauger/kakarot that referenced this pull request Nov 9, 2024
* bump to v0.8.9

* chore: bump deps (kkrt-labs#1269)

<!--- Please provide a general summary of your changes in the title
above -->

<!-- Give an estimate of the time you spent on this PR in terms of work
days.
Did you spend 0.5 days on this PR or rather 2 days?  -->

Time spent on this PR: 0.3d

## Pull request type

<!-- Please try to limit your pull request to one type,
submit multiple pull requests if needed. -->

Please check the type of change your PR introduces:

- [ ] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe):

## What is the current behavior?

<!-- Please describe the current behavior that you are modifying,
or link to a relevant issue. -->

Resolves #<Issue number>

## What is the new behavior?

<!-- Please describe the behavior or changes that are being added by
this PR. -->

- Bumps dependencies, required to use the latest types in the cairo core
lib.
- Bumping starknet-py requires bumping Katana to v1.0, which supports
the latest rpc spec 0.7.1
- Instead of relying on a "hardcoded" erc20 json abi, add the contract
to the list of contracts to compile

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg"
height="34" align="absmiddle"
alt="Reviewable"/>](https://reviewable.io/reviews/kkrt-labs/kakarot/1269)
<!-- Reviewable:end -->

---------

Co-authored-by: Clément Walter <clement0walter@gmail.com>

* fetch the starknet block number of `eth_blockNumber`

* ignore mempool tests

---------

Co-authored-by: Clément Walter <clement0walter@gmail.com>
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.

bug: missing range_check assertion for RLP.decode
2 participants