Stylus SDK 0.7 Audit

Table of Contents

Summary

Type

Smart Contract Language

Timeline

From: 2024-11-04

To: 2024-11-22

Languages

Rust

Total Issues

18 (0 resolved)

Critical Severity Issues

1 (0 resolved)

High Severity Issues

2 (0 resolved)

Medium Severity Issues

1 (0 resolved)

Low Severity Issues

6 (0 resolved)

Notes & Additional Information

8 (0 resolved)

Client Reported Issues

0 (0 resolved)

Scope

We audited the OffchainLabs/stylus-sdk-rs repository at the commit f56fc6b. This was a diff audit starting from commit 62bd831.

In scope were all files under the stylus-proc, stylus-sdk, mini-alloc, and examples directories.

Overview of Changes

While not exhaustive, this release is comprised of three major changes/features:

Implementation of fallback/receive

Previous versions of the Stylus SDK did not support the special functions fallback and receive. In the current iteration, they are supported by adding either the #[fallback] or #[recieve] attribute to any public function within a Stylus contract, as long as they match the expected function signature of the designated special function.

Support for all integer types of alloy-primitives

The previous version of the SDK only allowed the alloy integer type U256 to be used in the external ABI, i.e., in function parameters or return types. They have since extended to support alloy_primitives::Signed and alloy_primitives::Uint in return types and parameters. This allows all integer sizes that are multiples of 8 bits to become available.

Refactoring of procedural macros

This version of the SDK introduces a major refactor of the Stylus procedural macros, focusing on improving maintainability, testability, and documentation. Key changes include transitioning from raw tokens to AST nodes for better unit testing and composability, adopting the visitor pattern to simplify nested loops, and organizing export-ABI code with extension traits for clarity. Error reporting is enhanced to show multiple errors simultaneously with consistent messaging. New testing utilities, such as assert_ast_eq for AST comparisons, are added, along with integration tests for compiled macros and failure tests for error cases. Documentation is improved through docstring updates and included in the test process, while a new README.md guides developers on maintaining these macros. Other updates include support for custom path separators in sol_interface!, a reorganized folder structure for macro implementations, and utilities for handling inner attributes.

Critical Severity

Miscalculation of Needed Bytes Causes Corrupt Storage Value

The set_uint function is part of the GlobalStorage trait, which manages storage interactions. This function is responsible for writing unsigned integers (Uint ) to storage. It operates by dividing the provided integer into bytes, determining its placement within a 32-byte storage slot, and copying the value into the correct offset within the storage slot. This is a crucial operation that is not only used to store unsigned integers but also handles storing signed integers and bytes [1] [2].

However, the function has a limitation when dealing with types whose bit-length B is not a multiple of 8. Specifically, the calculation B / 8 truncates to the nearest integer, which can result in fewer bytes being copied than are necessary for proper storage. For example, if B = 39, B / 8 evaluates to 4, but 39 bits require 5 bytes. As a result, when storing a Uint<39, 1>, only the first 4 bytes are written, causing the data to be shifted 8 bits to the right, and the rightmost byte is lost. Consequently, any custom type using less than 8 bits will always store a value of 0. This can have devastating consequences for a project using unconventional types to handle ETH balances, time values, or any other sensitive data.

Furthermore, allowing the use of custom types for storage variables is incompatible with Solidity's storage. This could cause an additional issue when migrating an upgradable Stylus contract that uses incompatible storage types to a Solidity implementation.

To ensure storage compatibility and reduce risk exposure, consider preventing storage declaration of integer types with bit-lengths that are not a multiple of 8 to guarantee compatibility with Solidity's ABI. Alternatively, if this behavior is intentional, consider resolving the incorrect calculation, taking bit-lengths not divisible by 8 into account. This ensures that the function copies all necessary bytes into storage, preventing truncation and preserving the integrity of the stored data.

High Severity

Potentially Unreachable fallback Function

The router_entrypoint function is responsible for routing the transaction input depending on the calldata and the message value sent along with the transaction, mimicking Solidity's behavior. When the input is empty, the function will try to call either the receive or fallback function. When the input is not empty, the function will attempt to find a function whose signature hash matches the first 4 bytes of the input calldata. If no selector is found, the function will attempt to call the fallback function, and if none is found an "unknown method selector" error will be returned.

However, if the input length is less than 4 bytes, the function returns a “calldata too short” error. This unexpected behavior disrupts the intended similarity to Solidity, making the fallback function unreachable when the calldata length is designed to be between 1 and 3 bytes.

To align with Solidity’s behavior, consider routing calldata lengths between 1 and 3 bytes to the fallback function.

Falsely Restricted Definitions for Special Functions

Solidity’s special functions have strict limitations on the types and number of input and output parameters they can accept. For instance, the receive function accepts no arguments, does not return any value, and is always payable. The fallback function, however, is more flexible and offers two possible implementations. It can be either declared without input and return parameters or with input bytes calldata and return bytes memory. The fallback function is optionally payable.

However, the current implementation of the fallback and the receive functions in Stylus contain discrepancies with Solidity.

For instance, the fallback function only allows taking in bytes in calldata and returns Option instead of Bytes. Additionally, there is no option to define a fallback function that takes no input and returns no output.

Moreover, the deny_value function is responsible for checking the purity of a function and deciding whether or not the function accepts Ether alongside the function call. This is done by checking if purity is Payable and returning None, without additional checks to ensure msg::value is empty, otherwise throwing an error if the function received Ether.

However, this check is not implemented for the fallback function, rendering the fallback function always payable. This could be an issue for developers that assume the fallback function is only payable when adding the #[payable] attribute; Ether could get stuck in the contract when users call the fallback function.

Furthermore, the receive function is set to return Option instead of returning nothing.

To conclude, the current implementation of special functions in Stylus is inconsistent with Solidity’s established behavior, which could lead to unexpected results and usability issues. Consider aligning the definitions and behavior of special functions with Solidity’s standards. This includes supporting both variations of the fallback function, enforcing proper checks for payability, and ensuring the receive function strictly adheres to Solidity’s rules by not returning any value. This will enhance the developer experience and mitigate risks of unintended behavior.

Medium Severity

Missing Override Check For Return Types

The Stylus SDK does not enforce consistency in return types between overriding functions and their parent definitions. In Solidity, the compiler ensures that the return types of overriding functions strictly match the parent's signature. This behavior is crucial for maintaining consistency and predictability in contract interactions. Contracts relying on standardized interfaces may no longer adhere to expected behaviors, making them incompatible with other contracts expecting specific return types. Consider adding override checks for return types to avoid incompatibility issues and conform to Solidity's behavior.

Low Severity

Inconsistent Override Checks for Special Functions

When ensuring that public functions follow safe override rules in overrides.rs, both selector_consts and override_arms use the fallback and receive functions' signatures to ensure that overriding fallback or receive functions follows the override rules.

Since Stylus does not enforce a specific name for the fallback and receive functions, a child contract inheriting from a parent and overriding its fallback function could use a different name, thus using a different selector hash. This allows users to lower the state mutability of the parent's function and bypass the override checks. For example, a payable fallback can be changed to a nonpayable state mutability. This could lead to confusion and unintended behavior as the users will likely trust the compiler to execute those checks correctly.

In the case of the receive function, there is only one allowed way to declare the function's input parameters, state mutability, and return parameters, making it unnecessary to check the correctness of the override function since it will always be valid.

Consider updating the override checks for the fallback function to validate the override using the function marked as fallback instead of relying on the function selectors. In the case of the receive function, consider removing the unnecessary checks for the changed state mutability.

Incomplete List of Solidity Keywords

The stylus-proc documentation in the lib.rs file suggests that arguments like address can be used without fear because they will get prepended by an _ when exporting the Solidity ABI. This statement implies that the Stylus SDK handles all possible keywords as well as reserved keywords. This behavior is implemented in the underscore_if_sol function.

However, the list of checked keywords and reserved keywords is not exhaustive. Many keywords, for example, function, external, internal, public, private, mapping, pragma, and others, are not checked. Moreover, the list of keywords and reserved keywords can change between Solidity versions, which can result in wrong error messages or not detecting newly added keywords in future versions.

Additionally, these checks are only applied for function parameter names. Function names, however, are unrestricted. In this case, the ABI generated for interaction with Solidity contracts may contain illegal values, making it impossible to compile and use the generated interfaces.

Implementing the keywords check in the SDK demands an extensive list for each Solidity version and can be error-prone. To solve these issues, consider generating a Solidity interface for each compilation, compiling the interface using a Solidity compiler, and returning the errors from the compiler accordingly instead of prepending arguments and allowing unrestricted function names. Consequently, this will improve maintainability and ensure compatibility of Stylus contracts with Solidity by outsourcing these checks to the compiler, which can behave differently between versions.

Struct Definitions Not Included in Interface Generated by export-abi

When the export-abi feature is turned on, the Stylus SDK will generate a Solidity interface for the entrypoint contract. Structs defined within an alloy sol! block may be used as parameter and return types in external functions in Stylus contracts by adding the #[derive(AbiType)] attribute, but the generated Solidity ABI will not include the definition of the struct. This will cause compilation to fail when importing the generated ABI into a Solidity project. For example, The following Stylus code:

 sol! {
 #[derive(AbiType)]
 struct Foo {
 uint256 bar;
 }
}
 pub fn function_with_struct_argument(
 &mut self,
 foo: Foo
 ) {
 // code goes here
}

will generate an ABI similar to this with cargo stylus export-abi:

 // struct `Foo` is missing from exported ABI
function functionWithStructArgument(Foo foo) external;

Consider including the struct definitions in the generated ABI to streamline the developer experience and avoid issues when importing ABIs into external Solidity projects.

Multiple Fallback and/or Receive Attributes Are Possible in the Same Contract

Users may add each of the #[fallback] and #[receive] attributes to more than one function in a contract. The resulting function used for the fallback or receive after macro expansion will be the one defined first in the contract using the attribute. To make development less error-prone, consider adding an error message and only allowing a single #[fallback] or #[receive] attribute to be added to a contract.

Incorrect Dependency Version in Examples

The example crates in the Stylus SDK currently depend on older versions of alloy-primitives and alloy-sol-types than the ones used in the parent SDK folder. The examples thus fail to compile. Consider aligning these and any further dependency mismatches across all crates to avoid errors.

Incompatible ABI Generated When Using Tuple Arguments

The Stylus SDK allows tuple arguments in functions definitions. When a contract's ABI is exported, the resulting interface will be incompatible with Solidity. For example, a function in a Stylus contract such as

 pub fn function_with_tuple_argument(&mut self, tuple: (u8, U256)) { ... }

will be exported in the Solidity interface as

 function functionWithTupleArgument((uint8, uint256) tuple) external;

which will be incompatible and cause compilation to fail if the generated interface is imported into a Solidity project. Similarly, custom Error types using tuple parameters in the SDK will be generated in the interface in a similar way and also be incompatible. While Solidity code does not allow tuple types to be used, they are used as the the internal encoding for structs within the ABI. Hence, the Stylus contract can still be called from Solidity, but the tuple needs to be represented as a struct in interfaces generated with export-abi. Consider refactoring the code generation logic around tuple types to ensure the exported interface is compatible and can be safely imported into external Solidity projects.

Notes & Additional Information

Redundant Computation of Function Selectors

When generating the function selector dispatcher, the selector_consts are computed to return all the function selector hashes of the functions of a contract.

Additionally, in the implementation of override checks, the selector_consts are generated twice on lines 39 and 53 in the impl_override_checks function. This means that each contract will have three identical declarations of the selector constants, one in the function dispatcher and two in the override checks implementation. This redundant code increases contract size.

Since these values are constants, consider declaring them in the root of the implementation once to make the generated code more concise and reduce the contract size.

Exported ABI Lacks Special Functions

The code generation executed when exporting the ABI in export_abi.rs is responsible for converting Stylus contracts into a Solidity interface. Solidity interfaces can be used either as a contract's ABI or as a design template for other contracts to inherit from and ensure all functions are implemented correctly, respecting the interface's ABI. This helps with standardization and facilitates interoperability.

While the export-abi feature only includes errors and functions, special functions (i.e., fallback and receive) are omitted during code generation. This makes the exported interfaces only suitable for contract interaction, but not for design implementation when special functions are part of the initial contract from which the interface was generated.

Consider including special functions in exported ABIs, extending the usability of the exported interfaces. Alternatively, consider documenting the limitations when special functions are part of the initial contract in the docs as well as extending the comments at the top of the exported interface, highlighting potentially missing special functions.

Missing Warnings for fallback and receive Function Names

In Solidity, fallback and receive are valid function names, i.e., it is allowed to have a fallback or receive function and a function fallback or function receive in a single contract. This is because the special functions fallback and receive are not invoked by checking a function selector like regular functions but rather using other logic to define which one to call. However, the Solidity compiler will raise a warning when compiling the contract, clarifying that the functions called fallback or receive are not native special functions.

Consider raising a warning when compiling a contract that contains a function called fallback or receive to clarify for the user the potential confusion around those special function names, advising them to use either #[fallback] or #[receive] if they intend to use either of the special functions.

Missing Documentation for Internal Pure Function Usage

In the Stylus SDK, a function will be assigned Pure mutability if the function does not include a &self or &mut self as the first argument. In Rust, this then becomes an associated function, meaning it is not tied to any specific instance of the struct that is implemented by the block containing the function. For developers coming from a Solidity background, this may be confusing as the function requires different syntax to call. For example, to access the function from within the same implementation block one would need Self::function, or if calling from a parent contract, Parent::function. This is different from the patterns used to call View or Writable functions within contract implementations. Consider documenting this caveat to avoid confusion for developers.

Outdated References to external Macro

On lines 19 and 21 in stylus-sdk/src/abi/export/mod.rs, there are still references to using the external macro, which has been deprecated in the SDK and users should instead use #[public]. Consider correcting these comments, along with any other references in the codebase, to improve overall clarity.

Undocumented Fallback and Receive Usage

The user may designate a function as fallback or receive in the Stylus SDK by using the #[fallback] and #[receive] attributes. However, this pattern is not documented anywhere in the code, in contrast with other common attributes such as #[payable]. Consider adding clear documentation to eliminate confusion around how the Stylus SDK implements fallback and receive.

Misleading Error Message in Override Checks When Functions Have Different Names But Same Selector

The Stylus SDK currently checks that overriding functions in a child contract are allowed based on rules given by the parent function's purity, which follows the same rules as in Solidity. When the parent and child function have different names but the same selector (for example, by using the custom selector feature in Stylus), the generated error message can be misleading. For example, if we consider a parent function f overridden by a child function g, with f of purity View and g of purity Write, the error message will incorrectly say "function g cannot be overridden with function marked Write", where in this context the message should be referring to f. Consider modifying the assert_override logic to account for this special case.

Automatic Case Conversion Potentially Conflicts with Function Naming

The Solidity Style Guide for function names recommends using the mixedCase (also called camelCase) format. Stylus, on the other hand, adheres to Rust’s snake_case naming convention. However, public functions are automatically converted to camelCase to align with the Solidity Style Guide. This approach can create issues when functions are deliberately written in a different casing.

For example, the EIP-2612 permit extension for EIP-20 defines a public DOMAIN_SEPARATOR() function, which would be renamed to domainSeparator(), breaking compatibility with the EIP’s interface. Developers must then use the selector attribute, which is initially intended to bypass public functions signature overloading, to preserve the original function name, potentially causing confusion when identical function and selector names are seen.

Consider adding the option to bypass case conversion entirely or expanding the documentation for the selector attribute to clarify its capabilities.

Conclusion

The latest iteration of the Stylus SDK adds support for fallback and receive functions, enables use of all integer types of alloy-primitives, and refactors the Stylus procedural macros to improve maintainability, efficiency, and code clarity. While all supported integer types must be multiples of 8 bits within a contract's ABI, integers with bit-length non-divisible by 8 are allowed in contract storage, a critical issue, leading to storage corruption. Two high severity issues related to incorrect behavior of special functions were also discovered. The codebase was well-written but could benefit from more documentation in terms of the intended usage of all features from a developer perspective. Despite this, it is exciting to see how rapidly the Stylus SDK has progressed and the thoughtfulness of the team in addressing all identified issues and improving the quality and functionality of the SDK overall.