fix-security #42

Merged
Skipper merged 3 commits from fix-security into main 2026-04-02 16:34:07 +00:00
7 changed files with 93 additions and 23 deletions

View File

@@ -214,7 +214,6 @@ impl KeyHolder {
let mut conn = self.db.get().await?;
schema::root_key_history::table
.filter(schema::root_key_history::id.eq(*root_key_history_id))
.select(schema::root_key_history::data_encryption_nonce)
.select(RootKeyHistory::as_select())
.first(&mut conn)
.await?

View File

@@ -210,12 +210,15 @@ where
}
};
if valid {
if !valid {
error!("Invalid challenge solution signature");
return Err(Error::InvalidChallengeSolution);
}
self.transport
.send(Ok(Outbound::AuthSuccess))
.await
.map_err(|_| Error::Transport)?;
}
Ok(key.clone())
}

View File

@@ -91,6 +91,7 @@ async fn query_relevant_past_transaction(
async fn check_rate_limits(
grant: &Grant<Settings>,
current_transfer_value: U256,
db: &mut impl AsyncConnection<Backend = Sqlite>,
) -> QueryResult<Vec<EvalViolation>> {
let mut violations = Vec::new();
@@ -99,12 +100,12 @@ async fn check_rate_limits(
let past_transaction = query_relevant_past_transaction(grant.id, window, db).await?;
let window_start = chrono::Utc::now() - grant.settings.limit.window;
let cumulative_volume: U256 = past_transaction
let prospective_cumulative_volume: U256 = past_transaction
.iter()
.filter(|(_, timestamp)| timestamp >= &window_start)
.fold(U256::default(), |acc, (value, _)| acc + *value);
.fold(current_transfer_value, |acc, (value, _)| acc + *value);
if cumulative_volume > grant.settings.limit.max_volume {
if prospective_cumulative_volume > grant.settings.limit.max_volume {
violations.push(EvalViolation::VolumetricLimitExceeded);
}
@@ -141,7 +142,7 @@ impl Policy for EtherTransfer {
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
}
let rate_violations = check_rate_limits(grant, db).await?;
let rate_violations = check_rate_limits(grant, meaning.value, db).await?;
violations.extend(rate_violations);
Ok(violations)

View File

@@ -198,7 +198,7 @@ async fn evaluate_rejects_volume_over_limit() {
grant_id,
wallet_access_id: WALLET_ACCESS_ID,
chain_id: CHAIN_ID as i32,
eth_value: utils::u256_to_bytes(U256::from(1_001u64)).to_vec(),
eth_value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
signed_at: SqliteTimestamp(Utc::now()),
})
.execute(&mut *conn)
@@ -211,7 +211,7 @@ async fn evaluate_rejects_volume_over_limit() {
shared: shared(),
settings,
};
let context = ctx(ALLOWED, U256::from(100u64));
let context = ctx(ALLOWED, U256::from(1u64));
let m = EtherTransfer::analyze(&context).unwrap();
let v = EtherTransfer::evaluate(&context, &m, &grant, &mut *conn)
.await
@@ -233,13 +233,13 @@ async fn evaluate_passes_at_exactly_volume_limit() {
.await
.unwrap();
// Exactly at the limit — the check is `>`, so this should not violate
// Exactly at the limit including current transfer — check is `>`, so this should not violate
insert_into(evm_transaction_log::table)
.values(NewEvmTransactionLog {
grant_id,
wallet_access_id: WALLET_ACCESS_ID,
chain_id: CHAIN_ID as i32,
eth_value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
eth_value: utils::u256_to_bytes(U256::from(900u64)).to_vec(),
signed_at: SqliteTimestamp(Utc::now()),
})
.execute(&mut *conn)

View File

@@ -101,6 +101,7 @@ async fn query_relevant_past_transfers(
async fn check_volume_rate_limits(
grant: &Grant<Settings>,
current_transfer_value: U256,
db: &mut impl AsyncConnection<Backend = Sqlite>,
) -> QueryResult<Vec<EvalViolation>> {
let mut violations = Vec::new();
@@ -113,12 +114,12 @@ async fn check_volume_rate_limits(
for limit in &grant.settings.volume_limits {
let window_start = chrono::Utc::now() - limit.window;
let cumulative_volume: U256 = past_transfers
let prospective_cumulative_volume: U256 = past_transfers
.iter()
.filter(|(_, timestamp)| timestamp >= &window_start)
.fold(U256::default(), |acc, (value, _)| acc + *value);
.fold(current_transfer_value, |acc, (value, _)| acc + *value);
if cumulative_volume > limit.max_volume {
if prospective_cumulative_volume > limit.max_volume {
violations.push(EvalViolation::VolumetricLimitExceeded);
break;
}
@@ -163,7 +164,7 @@ impl Policy for TokenTransfer {
violations.push(EvalViolation::InvalidTarget { target: meaning.to });
}
let rate_violations = check_volume_rate_limits(grant, db).await?;
let rate_violations = check_volume_rate_limits(grant, meaning.value, db).await?;
violations.extend(rate_violations);
Ok(violations)

View File

@@ -220,7 +220,7 @@ async fn evaluate_rejects_wrong_restricted_recipient() {
}
#[tokio::test]
async fn evaluate_passes_volume_within_limit() {
async fn evaluate_passes_volume_at_exact_limit() {
let db = db::create_test_pool().await;
let mut conn = db.get().await.unwrap();
@@ -230,7 +230,7 @@ async fn evaluate_passes_volume_within_limit() {
.await
.unwrap();
// Record a past transfer of 500 (within 1000 limit)
// Record a past transfer of 900, with current transfer 100 => exactly 1000 limit
use crate::db::{models::NewEvmTokenTransferLog, schema::evm_token_transfer_log};
insert_into(evm_token_transfer_log::table)
.values(NewEvmTokenTransferLog {
@@ -239,7 +239,7 @@ async fn evaluate_passes_volume_within_limit() {
chain_id: CHAIN_ID as i32,
token_contract: DAI.to_vec(),
recipient_address: RECIPIENT.to_vec(),
value: utils::u256_to_bytes(U256::from(500u64)).to_vec(),
value: utils::u256_to_bytes(U256::from(900u64)).to_vec(),
})
.execute(&mut *conn)
.await
@@ -282,7 +282,7 @@ async fn evaluate_rejects_volume_over_limit() {
chain_id: CHAIN_ID as i32,
token_contract: DAI.to_vec(),
recipient_address: RECIPIENT.to_vec(),
value: utils::u256_to_bytes(U256::from(1_001u64)).to_vec(),
value: utils::u256_to_bytes(U256::from(1_000u64)).to_vec(),
})
.execute(&mut *conn)
.await
@@ -294,7 +294,7 @@ async fn evaluate_rejects_volume_over_limit() {
shared: shared(),
settings,
};
let calldata = transfer_calldata(RECIPIENT, U256::from(100u64));
let calldata = transfer_calldata(RECIPIENT, U256::from(1u64));
let context = ctx(DAI, calldata);
let m = TokenTransfer::analyze(&context).unwrap();
let v = TokenTransfer::evaluate(&context, &m, &grant, &mut *conn)

View File

@@ -165,3 +165,69 @@ pub async fn test_challenge_auth() {
task.await.unwrap().unwrap();
}
#[tokio::test]
#[test_log::test]
pub async fn test_challenge_auth_rejects_invalid_signature() {
let db = db::create_test_pool().await;
let actors = GlobalActors::spawn(db.clone()).await.unwrap();
let new_key = ed25519_dalek::SigningKey::generate(&mut rand::rng());
let pubkey_bytes = new_key.verifying_key().to_bytes().to_vec();
// Pre-register key with key_type
{
let mut conn = db.get().await.unwrap();
insert_into(schema::useragent_client::table)
.values((
schema::useragent_client::public_key.eq(pubkey_bytes.clone()),
schema::useragent_client::key_type.eq(1i32),
))
.execute(&mut conn)
.await
.unwrap();
}
let (server_transport, mut test_transport) = ChannelTransport::new();
let db_for_task = db.clone();
let task = tokio::spawn(async move {
let mut props = UserAgentConnection::new(db_for_task, actors);
auth::authenticate(&mut props, server_transport).await
});
test_transport
.send(auth::Inbound::AuthChallengeRequest {
pubkey: AuthPublicKey::Ed25519(new_key.verifying_key()),
bootstrap_token: None,
})
.await
.unwrap();
let response = test_transport
.recv()
.await
.expect("should receive challenge");
let challenge = match response {
Ok(resp) => match resp {
auth::Outbound::AuthChallenge { nonce } => nonce,
other => panic!("Expected AuthChallenge, got {other:?}"),
},
Err(err) => panic!("Expected Ok response, got Err({err:?})"),
};
// Sign a different challenge value so signature format is valid but verification must fail.
let wrong_challenge = arbiter_proto::format_challenge(challenge + 1, &pubkey_bytes);
let signature = new_key.sign(&wrong_challenge);
test_transport
.send(auth::Inbound::AuthChallengeSolution {
signature: signature.to_bytes().to_vec(),
})
.await
.unwrap();
assert!(matches!(
task.await.unwrap(),
Err(auth::Error::InvalidChallengeSolution)
));
}