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

Replace unsafe arithmetic with checked_add #2448

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

nickgarfield
Copy link
Contributor

We run an automated auditor on our contracts. After upgrading to 0.27.0, we started seeing this error show up in the reports. There is an unsafe arithmetic operation in the __idl_write function. This PR updates it to use checked_sum instead.

Analyzing /home/ubuntu/clockwork/programs/thread/.coderrect/build/bpfel-unknown-unknown/release/all.ll ...
Cargo.toml: anchor_lang version: 0.27.0
anchor_lang_version: 0.27.0 anchorVersionTooOld: 0
Cargo.toml: anchor_lang version: 0.27.0
anchor_lang_version: 0.27.0 anchorVersionTooOld: 0
 - ✔ [00m:03s] Loading IR From File
 - ▘ [00m:00s] Running Compiler Optimization Passes
EntryPoints:
entrypoint
 - ✔ [00m:00s] Running Compiler Optimization Passes
 - ✔ [00m:01s] Running Pointer Analysis
=============This arithmetic operation may be UNSAFE!================
Found a potential vulnerability at line 23, column 1 in programs/thread/src/lib.rs
The add operation may result in overflows:

 17|use instructions::*;
 18|use state::*;
 19|
 20|declare_id!("CLoCKyJ6DXBJqqu2VWx9RLbgnwwR6BMHHuyasVmfMzBh");
 21|
 22|/// Program for creating transaction threads on Solana.
>23|#[program]
 24|pub mod thread_program {
 25|    use super::*;
 26|
 27|    /// Return the crate information via `sol_set_return_data/sol_get_return_data`
 28|    pub fn get_crate_info(ctx: Context<GetCrateInfo>) -> Result<CrateInfo> {
 29|        get_crate_info::handler(ctx)
>>>Stack Trace:
>>>clockwork_thread_program::try_entry::hbb84678ed52dface [programs/thread/src/lib.rs:23]
>>>  clockwork_thread_program::dispatch::h293401d576feb4f4 [programs/thread/src/lib.rs:23]
>>>    clockwork_thread_program::__private::__idl::__idl_dispatch::haf1f701bdedf3b7b [programs/thread/src/lib.rs:23]
>>>      clockwork_thread_program::__private::__idl::__idl_write::hdbe58065e9110f8a [programs/thread/src/lib.rs:23]

 - ✔ [00m:02s] Building Static Happens-Before Graph
 - ✔ [00m:00s] Detecting Vulnerabilities
detected 0 untrustful accounts in total.
detected 1 unsafe math operations in total.

--------The summary of potential vulnerabilities in all.ll--------

(Note: Soteria checks 5 common vulnerabilities in Solana programs w/o Anchor;
for sec3's advanced X-ray scanner, please visit https://sec3.dev/x-ray)

	 1 unsafe arithmetic issues

@vercel
Copy link

vercel bot commented Mar 30, 2023

@nickgarfield is attempting to deploy a commit to the coral-xyz Team on Vercel.

A member of the Team first needs to authorize it.

@armaniferrante
Copy link
Member

lgtm, will let CI run

@callensm callensm merged commit fbd0688 into coral-xyz:master Mar 30, 2023
@kevinheavey
Copy link
Contributor

@nickgarfield is this Soteria? Is there a way to get it to ignore certain lines?

@nickgarfield
Copy link
Contributor Author

@nickgarfield is this Soteria? Is there a way to get it to ignore certain lines?

It is Soteria, but I'm not sure about the configs. Do you know of any cargo config that could set the checked math behavior as default for a workspace?

@kevinheavey
Copy link
Contributor

@nickgarfield yeah you can add this to the workspace Cargo.toml:

[profile.release]
overflow-checks = true

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.

4 participants