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

jolt::provable attribute omits subsequent attributes #588

Open
cre-mer opened this issue Feb 12, 2025 · 3 comments
Open

jolt::provable attribute omits subsequent attributes #588

cre-mer opened this issue Feb 12, 2025 · 3 comments
Assignees

Comments

@cre-mer
Copy link
Contributor

cre-mer commented Feb 12, 2025

The jolt::provable proc_macro_attribute does not take into account subsequent attributes when generating the token stream of a function. For instance, the make_execute_function's quote! does not output any used attributes alongside the function itself. This can lead to arbitrary behavior. If a user adds a custom safety-attribute or logical-attribute, this attribute will be omitted, which could introduce an unknown vulnerability or prevent the program from compiling. For example, the following code will fail to compile:

// guest code
#[jolt::provable]
#[allow(arithmetic_overflow)]
pub fn overflow_add() -> u8 {
    let x = u8::MAX + u8::MAX;
    x // should return 254
}

with the following error:

error: this arithmetic operation will overflow
  --> guest/src/lib.rs:21:13
   |
21 |     let x = u8::MAX + u8::MAX;
   |             ^^^^^^^^^^^^^^^^^ attempt to compute `u8::MAX + u8::MAX`, which would overflow
   |
   = note: `#[deny(arithmetic_overflow)]` on by default

error: could not compile `guest` (lib) due to 1 previous error

In this case, this issue is preventing the program from compiling, but other attributes if omitted could introduce undefined behavior.

Consider either allowing the usage of further attributes, or parsing all function attributes and whitelisting a defined subset of those attributes to prevent undefined behavior.

@cre-mer
Copy link
Contributor Author

cre-mer commented Feb 12, 2025

This pattern could also be present across other proc macros and should be checked carefully

@ncitron
Copy link
Contributor

ncitron commented Feb 13, 2025

Good catch. We should look to see if it is possible to copy over subsequent attributes into our functions that are generated internally.

@cre-mer
Copy link
Contributor Author

cre-mer commented Feb 13, 2025

This is possible by extracting the attributes and outputting them like so:

fn make_execute_function(&self) -> TokenStream2 {
        let fn_name = self.get_func_name();
        let inputs = &self.func.sig.inputs;
        let output = &self.func.sig.output;
        let body = &self.func.block;
+       let attr = &self.func.attrs;

        quote! {
            #[cfg(not(target_arch = "wasm32"))]
+           #(#fn_attrs)* // retain other macros
             pub fn #fn_name(#inputs) #output {
                 #body
             }
        }
    }

However, there could be valid cases where you want to prevent other attributes or whitelist other attributes. Attribute ordering can also play a role – I also noticed that preceding jold::provable with an attribute can also cause failure. For example:

// guest code
#[allow(arithmetic_overflow)]
#[jolt::provable]
pub fn overflow_add() -> u8 {
    let x = u8::MAX + u8::MAX;
    x // should return 254
}

also fails with:

error: this arithmetic operation will overflow
  --> guest/src/lib.rs:21:13
   |
21 |     let x = u8::MAX + u8::MAX;
   |             ^^^^^^^^^^^^^^^^^ attempt to compute `u8::MAX + u8::MAX`, which would overflow
   |
   = note: `#[deny(arithmetic_overflow)]` on by default

error: could not compile `guest` (lib) due to 1 previous error

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

No branches or pull requests

2 participants