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

big_mod_exp syscall: exclude inputs of numbers larger than 4096-bits #32520

Merged
merged 5 commits into from
Jul 18, 2023
Merged

big_mod_exp syscall: exclude inputs of numbers larger than 4096-bits #32520

merged 5 commits into from
Jul 18, 2023

Conversation

samkim-crypto
Copy link
Contributor

Problem

The changes related to Big integer modular exponentiation (EIP-198). The calculation is implemented as syscall.

The formula that was used to approximate the compute budgets for large number exponentiation seems to under-approximate the costs for exponentiation of very large numbers (i.e. >>1000 byte numbers). This could lead to a negative impact on the network if the syscall was to be invoked with very large numbers.
In practice, it seems like any normal/non-adversarial use of the syscall will be on numbers of size less than or equal to 4096-bits (i.e. for RSA-4096)

Summary of Changes

Inside syscall all inputs are limited by value 4069 bits (512 bytes).
If the length is exceeded, an error SyscallError::InvalidLength returned.

This is a combination of the fixes by @valiksinev and the tests by @derrekr and subsumes #31859.

Fixes #

@codecov
Copy link

codecov bot commented Jul 18, 2023

Codecov Report

Merging #32520 (c1cc432) into master (c69bc00) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master   #32520    +/-   ##
========================================
  Coverage    82.0%    82.0%            
========================================
  Files         779      779            
  Lines      210609   210649    +40     
========================================
+ Hits       172742   172848   +106     
+ Misses      37867    37801    -66     

@samkim-crypto samkim-crypto requested a review from t-nelson July 18, 2023 12:35
@mvines mvines added the v1.16 PRs that should be backported to v1.16 label Jul 18, 2023
Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

:shipit:

@samkim-crypto samkim-crypto merged commit 5d9c1d8 into solana-labs:master Jul 18, 2023
mergify bot pushed a commit that referenced this pull request Jul 18, 2023
…32520)

* limit inputs length

* fix clippy

* add test_syscall_big_mod_exp test

* cargo fmt

* add SBPFVersion

---------

Co-authored-by: valiksinev <[email protected]>
Co-authored-by: derrek <[email protected]>
(cherry picked from commit 5d9c1d8)
mvines pushed a commit that referenced this pull request Jul 19, 2023
…6-bits (backport of #32520) (#32526)

big_mod_exp syscall: exclude inputs of numbers larger than 4096-bits (#32520)

* limit inputs length

* fix clippy

* add test_syscall_big_mod_exp test

* cargo fmt

* add SBPFVersion

---------

Co-authored-by: valiksinev <[email protected]>
Co-authored-by: derrek <[email protected]>
(cherry picked from commit 5d9c1d8)

Co-authored-by: samkim-crypto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants