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

solana: Configure clippy to deny possible truncation #285

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/solana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
run: cargo check --workspace --tests --manifest-path Cargo.toml

- name: Run `cargo clippy`
run: cargo clippy --workspace --tests --manifest-path Cargo.toml
run: cargo clippy --workspace --tests --manifest-path Cargo.toml -- -Dclippy::cast_possible_truncation

- name: Cache solana tools
id: cache-solana
Expand Down
3 changes: 1 addition & 2 deletions solana/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@ warnings = "deny"

# 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_possible_truncation = "deny"
# cast_lossless= "deny"
# as_conversions = "deny"
# arithmetic_side_effects = "deny"
Expand Down
2 changes: 0 additions & 2 deletions solana/programs/example-native-token-transfers/src/bitmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ impl Bitmap {

pub fn count_enabled_votes(&self, enabled: Bitmap) -> u8 {
let bm = BM::<128>::from_value(self.map) & BM::<128>::from_value(enabled.map);
// Conversion from usize to u8 is safe here. The Bitmap uses u128, so its maximum length
// (number of true bits) is 128.
bm.len()
.try_into()
.expect("Bitmap length must not exceed the bounds of u8")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,15 @@ 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.
// 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.
// SECURITY: Truncation is allowed here. Clippy warns about the final returned expression, but it is
// safe.
#[allow(clippy::integer_division)]
#[allow(clippy::cast_sign_loss)]
#[allow(clippy::cast_possible_truncation)]
pub fn capacity_at(&self, now: UnixTimestamp) -> u64 {
assert!(self.last_tx_timestamp <= now);

Expand All @@ -76,6 +79,10 @@ impl RateLimitState {
+ time_passed as u128 * limit / (Self::RATE_LIMIT_DURATION as u128)
};

// The use of `min` here prevents truncation.
// The value of `limit` is u64 in reality. If both `calculated_capacity` and `limit` are at
// their maxiumum possible values (u128::MAX and u64::MAX), then u64::MAX will be chosen by
// `min`. So truncation is not possible.
calculated_capacity.min(limit) as u64
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ impl GovernanceMessage {
acc.is_signer.write(writer)?;
acc.is_writable.write(writer)?;
}
(data.len() as u16).write(writer)?;
u16::try_from(data.len())
.map_err(|_| io::Error::new(io::ErrorKind::InvalidData, "data length overflow"))?
.write(writer)?;
writer.write_all(data)
}
}
Expand Down
Loading