solana: Configure additional security-relevant clippy lints

Adds a variety of clippy lints to enforce safe programming patterns
Adds allow annotations for known-safe examples and explanations as to
why they're OK
Modifies some minor violations in the codebase

Author: johnsaigle

solana/Cargo.toml

@@ -5,6 +5,39 @@ members = [
 ]
 resolver = "2"
 
+# Lints to improve security
+[workspace.lints.clippy]
+# The following rules should be enabled but conflict with other open PRs.
+# TODO enable the commented rules below
+# cast_possible_truncation = "deny"
+# cast_lossless= "deny"
+# as_conversions = "deny"
+# arithmetic_side_effects = "deny"
+# TODO: These rules should probably be enabled. Expect and unwrap are used in some traits and signal that something
+# has gone wrong with the program.
+# expect_used = "deny"
+# unwrap_used = "deny"
+integer_division = "deny"
+cast_possible_wrap = "deny"
+cast_precision_loss = "deny"
+cast_sign_loss = "deny"
+unchecked_duration_subtraction = "deny"
+overflow_check_conditional = "deny"
+modulo_one = "deny"
+manual_slice_size_calculation = "deny"
+large_stack_arrays = "deny"
+large_stack_frames = "deny"
+large_futures = "deny"
+recursive_format_impl = "deny"
+eq_op = "deny"
+lossy_float_literal = "deny"
+float_cmp = "deny"
+panic = "deny"
+todo = "deny"
+out_of_bounds_indexing = "deny"
+unreachable = "deny"
+cast_abs_to_unsigned = "deny"
+
 [workspace.dependencies]
 wormhole-io = "0.1.3"
 wormhole-solana-utils = "0.2.0-alpha.15"
@@ -28,3 +61,4 @@ codegen-units = 1
 opt-level = 3
 incremental = false
 codegen-units = 1
+

solana/modules/ntt-messages/Cargo.toml

@@ -9,6 +9,9 @@ wormhole = []
 hash = [ "solana-program" ]
 anchor = [ "anchor-lang" ]
 
+[lints]
+workspace = true
+
 [dependencies]
 anchor-lang = { workspace = true, optional = true }
 wormhole-io.workspace = true

solana/modules/ntt-messages/src/trimmed_amount.rs

@@ -50,6 +50,9 @@ impl TrimmedAmount {
         }
     }
 
+    // Integer division is allowed here. The purpose of using it here is to remove the remainder so
+    // there is no risk.
+    #[allow(clippy::integer_division)]
     fn scale(amount: u64, from_decimals: u8, to_decimals: u8) -> u64 {
         if from_decimals == to_decimals {
             return amount;

solana/programs/example-native-token-transfers/Cargo.toml

@@ -26,6 +26,9 @@ solana-devnet = [ "wormhole-anchor-sdk/solana-devnet" ]
 tilt-devnet = [ "wormhole-anchor-sdk/tilt-devnet" ]
 tilt-devnet2 = [ "tilt-devnet" ]
 
+[lints]
+workspace = true
+
 [dependencies]
 ahash = "=0.8.5"
 

solana/programs/example-native-token-transfers/src/instructions/transfer.rs

@@ -74,7 +74,7 @@ impl TransferArgs {
             amount.to_be_bytes().as_ref(),
             recipient_chain.id.to_be_bytes().as_ref(),
             recipient_address,
-            &[*should_queue as u8],
+            &[u8::from(*should_queue)],
         ])
     }
 }

solana/programs/example-native-token-transfers/src/queue/rate_limit.rs

@@ -46,11 +46,16 @@ impl RateLimitState {
     /// Returns the capacity of the rate limiter.
     /// On-chain programs and unit tests should always use [`capacity`].
     /// This function is useful in solana-program-test, where the clock sysvar
-    ///
+    /// SECURITY: Integer division is OK here. We are not that concerned with precision. Removing
+    /// the remainder in this case is arguably more secure as it reduces the available capacity.
+    /// SECURITY: Sign loss is OK here. It is a conversion performed on a timestamp that must always be
+    /// positive.
+    #[allow(clippy::integer_division)]
+    #[allow(clippy::cast_sign_loss)]
     pub fn capacity_at(&self, now: UnixTimestamp) -> u64 {
         assert!(self.last_tx_timestamp <= now);
 
-        let limit = self.limit as u128;
+        let limit = u128::from(self.limit);
 
         // morally this is
         // capacity = old_capacity + (limit / rate_limit_duration) * time_passed
@@ -67,7 +72,7 @@ impl RateLimitState {
 
         let calculated_capacity = {
             let time_passed = now - self.last_tx_timestamp;
-            capacity_at_last_tx as u128
+            u128::from(capacity_at_last_tx)
                 + time_passed as u128 * limit / (Self::RATE_LIMIT_DURATION as u128)
         };
 
@@ -127,6 +132,7 @@ mod tests {
     use super::*;
 
     #[test]
+    #[allow(clippy::integer_division)]
     fn test_rate_limit() {
         let now = current_timestamp();
         let mut rate_limit_state = RateLimitState {

solana/programs/ntt-quoter/Cargo.toml

@@ -19,6 +19,9 @@ solana-devnet = []
 tilt-devnet = []
 tilt-devnet2 = ["tilt-devnet"]
 
+[lints]
+workspace = true
+
 [dependencies]
 anchor-lang = { workspace = true, features = ["init-if-needed"] }
 solana-program.workspace = true

solana/programs/ntt-quoter/src/processor/request_relay.rs

@@ -58,6 +58,9 @@ const GWEI: u64 = u64::pow(10, 9);
 
 //TODO built-in u128 division likely still wastes a ton of compute units
 //     might be more efficient to use f64 or ruint crate
+// SECURITY: Integer division is OK here. The calling code is responsible for understanding that
+// this function returns the quotient of the operation and that the remainder will be lost.
+#[allow(clippy::integer_division)]
 fn mul_div(scalar: u64, numerator: u64, denominator: u64) -> u64 {
     if scalar > 0 {
         //avoid potentially expensive u128 division

solana/programs/wormhole-governance/Cargo.toml

@@ -23,6 +23,9 @@ solana-devnet = [ "wormhole-anchor-sdk/solana-devnet" ]
 tilt-devnet = [ "wormhole-anchor-sdk/tilt-devnet" ]
 tilt-devnet2 = [ "tilt-devnet" ]
 
+[lints]
+workspace = true
+
 [dependencies]
 anchor-lang.workspace = true
 solana-program.workspace = true