Summary
Note that no restrictions are applied when passing in the maxInactiveInterval parameter, so providing excessively large values can lead to integer overflow.
Details
https://github.com/rooch-network/rooch/blob/main/frameworks/rooch-framework/sources/session_key.move
public fun create_session_key(
sender: &signer,
app_name: std::string::String,
app_url: std::string::String,
authentication_key: vector<u8>,
scopes: vector<SessionScope>,
max_inactive_interval: u64) {
//Can not create new session key by the other session key
assert!(!auth_validator::is_validate_via_session_key(), ErrorSessionKeyCreatePermissionDenied);
let sender_addr = signer::address_of(sender);
assert!(!exists_session_key(sender_addr, authentication_key), ErrorSessionKeyAlreadyExists);
let now_seconds = timestamp::now_seconds();
let session_key = SessionKey {
app_name,
app_url,
authentication_key,
scopes,
create_time: now_seconds,
last_active_time: now_seconds,
max_inactive_interval,
};
if (!account::exists_resource<SessionKeys>(sender_addr)){
let keys = table::new<vector<u8>, SessionKey>();
account::move_resource_to<SessionKeys>(sender, SessionKeys{keys});
};
let session_keys = account::borrow_mut_resource<SessionKeys>(sender_addr);
table::add(&mut session_keys.keys, authentication_key, session_key);
}
In the create_session_key function, there's no restriction on the size of the max_inactive_interval parameter. Later, during transaction execution in the is_expired function, an expiration check is performed. When session_key.last_active_time is added to session_key.max_inactive_interval, a u64 overflow may occur, causing the transaction to fail.
public(friend) fun is_expired(session_key: &SessionKey) : bool {
let now_seconds = timestamp::now_seconds();
if (session_key.max_inactive_interval > 0 && session_key.last_active_time + session_key.max_inactive_interval < now_seconds){
return true
};
return false
}
PoC
use https://github.com/rooch-network/my-first-rooch-dapp
const defaultScopes = [`${counterAddress}::*::*`];
createSessionKey(
{
appName: "my_first_rooch_dapp",
appUrl: "http://localhost:5173",
maxInactiveInterval: 18446744073709551610n,
scopes: defaultScopes,
},
{
onSuccess: (result) => {
console.log("session key", result);
},
onError: (why) => {
console.log(why);
},
}
).finally(() => setSessionLoading(false));
};
step 1 createSessionKey
step2 click Increase Counter Value button
Impact
If there is no input limit or range check on the max_inactive_interval, a malicious user can deliberately provide an extremely large value. This could lead to an overflow during the expiration check, causing the program to abort and potentially resulting in a Denial of Service (DoS) scenario.
Fixed Code:
Implement numeric range restrictions and validations on max_inactive_interval across all functions.
baicaiyihao Reporter
Summary
Note that no restrictions are applied when passing in the maxInactiveInterval parameter, so providing excessively large values can lead to integer overflow.
Details
https://github.com/rooch-network/rooch/blob/main/frameworks/rooch-framework/sources/session_key.move
In the create_session_key function, there's no restriction on the size of the max_inactive_interval parameter. Later, during transaction execution in the is_expired function, an expiration check is performed. When session_key.last_active_time is added to session_key.max_inactive_interval, a u64 overflow may occur, causing the transaction to fail.
PoC
use https://github.com/rooch-network/my-first-rooch-dapp
step 1 createSessionKey
step2 click Increase Counter Value button
Impact
If there is no input limit or range check on the max_inactive_interval, a malicious user can deliberately provide an extremely large value. This could lead to an overflow during the expiration check, causing the program to abort and potentially resulting in a Denial of Service (DoS) scenario.
Fixed Code:
Implement numeric range restrictions and validations on max_inactive_interval across all functions.