Skip to content

Commit

Permalink
fix(CoSigner): decoding the ABI calldata from Safe format, PCI expira…
Browse files Browse the repository at this point in the history
…tion and revocation check. (#812)

* fix(CoSigner): decoding the ABI calldata with packed and unpacked formats

* fix: removing DynSolValue
  • Loading branch information
geekbrother authored Oct 17, 2024
1 parent a95cb88 commit 4ada030
Show file tree
Hide file tree
Showing 4 changed files with 180 additions and 175 deletions.
28 changes: 25 additions & 3 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,6 @@ pub enum RpcError {
#[error("IRN client is not configured")]
IrnNotConfigured,

#[error("Permission for PCI is not found: {0} {1}")]
PermissionNotFound(String, String),

#[error("Internal permissions get context error: {0}")]
InternalGetSessionContextError(InternalGetSessionContextError),

Expand All @@ -215,12 +212,21 @@ pub enum RpcError {
#[error("Pkcs8 error: {0}")]
Pkcs8Error(#[from] ethers::core::k256::pkcs8::Error),

#[error("Permission for PCI is not found: {0} {1}")]
PermissionNotFound(String, String),

#[error("Permission context was not updated yet: {0}")]
PermissionContextNotUpdated(String),

#[error("Permission is revoked: {0}")]
RevokedPermission(String),

#[error("Permission is expired: {0}")]
PermissionExpired(String),

#[error("Permissions set is empty")]
CoSignerEmptyPermissions,

#[error("Cosigner permission denied: {0}")]
CosignerPermissionDenied(String),

Expand Down Expand Up @@ -501,6 +507,14 @@ impl IntoResponse for RpcError {
)),
)
.into_response(),
Self::PermissionExpired(pci) => (
StatusCode::BAD_REQUEST,
Json(new_error_response(
"pci".to_string(),
format!("Permission is expired: {}", pci),
)),
)
.into_response(),
Self::WrongBase64Format(e) => (
StatusCode::BAD_REQUEST,
Json(new_error_response(
Expand All @@ -525,6 +539,14 @@ impl IntoResponse for RpcError {
)),
)
.into_response(),
Self::CoSignerEmptyPermissions => (
StatusCode::BAD_REQUEST,
Json(new_error_response(
"".to_string(),
"Permissions set is empty".to_string(),
)),
)
.into_response(),
Self::AbiDecodingError(e) => (
StatusCode::BAD_REQUEST,
Json(new_error_response(
Expand Down
24 changes: 22 additions & 2 deletions src/handlers/sessions/cosign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,30 @@ async fn handler_internal(
.add_irn_latency(irn_call_start, OperationType::Hget);
let storage_permissions_item =
serde_json::from_str::<StoragePermissionsItem>(&storage_permissions_item)?;
let call_data = request_payload.user_op.call_data.clone();

// Check if the permission is revoked
if storage_permissions_item.revoked_at.is_some() {
return Err(RpcError::RevokedPermission(request_payload.pci.clone()));
}

// Check if the permission is expired
if storage_permissions_item.expiry
< SystemTime::now()
.duration_since(SystemTime::UNIX_EPOCH)
.unwrap_or_default()
.as_secs() as usize
{
return Err(RpcError::PermissionExpired(request_payload.pci.clone()));
}

let call_data = request_payload.user_op.call_data.clone();
// Extract the batch components
let execution_batch = extract_execution_batch_components(call_data.to_vec())?;
let execution_batch = extract_execution_batch_components(&call_data)?;

// Check the permissions length
if storage_permissions_item.permissions.is_empty() {
return Err(RpcError::CoSignerEmptyPermissions);
}

// Check permissions by types
for permission in storage_permissions_item.permissions {
Expand Down
22 changes: 8 additions & 14 deletions src/utils/permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ use {
crate::{
error::RpcError,
utils::sessions::{
extract_addresses_from_execution_batch, extract_values_from_execution_batch,
extract_addresses_from_execution_batch, extract_values_sum_from_execution_batch,
ExecutionTransaction,
},
},
ethers::{
abi::Token,
types::{H160, U256},
},
alloy::primitives::{Address, U256},
serde::{Deserialize, Serialize},
serde_json::Value,
strum_macros::{Display, EnumIter, EnumString},
Expand All @@ -27,7 +25,7 @@ pub enum PermissionType {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct ContractCallPermissionData {
pub address: H160,
pub address: Address,
pub abi: Value,
pub functions: Value,
}
Expand All @@ -43,10 +41,10 @@ pub struct NativeTokenTransferPermissionData {

/// `contract-call` permission type check
pub fn contract_call_permission_check(
execution_batch: Vec<Token>,
execution_batch: Vec<ExecutionTransaction>,
contract_call_permission_data: ContractCallPermissionData,
) -> Result<(), RpcError> {
let execution_addresses = extract_addresses_from_execution_batch(execution_batch.clone())?;
let execution_addresses = extract_addresses_from_execution_batch(execution_batch)?;
let call_address = contract_call_permission_data.address;

for address in execution_addresses {
Expand All @@ -62,15 +60,11 @@ pub fn contract_call_permission_check(

/// `native-token-transfer` permission type check
pub fn native_token_transfer_permission_check(
execution_batch: Vec<Token>,
execution_batch: Vec<ExecutionTransaction>,
native_token_transfer_permission_data: NativeTokenTransferPermissionData,
) -> Result<(), RpcError> {
let execution_values = extract_values_from_execution_batch(execution_batch.clone())?;
let allowance = native_token_transfer_permission_data.allowance;
// summ execution values from the execution batch and check if it is less than or equal to the allowance
let sum: U256 = execution_values
.iter()
.fold(U256::zero(), |acc, &x| acc + x);
let sum: U256 = extract_values_sum_from_execution_batch(execution_batch)?;
if sum > allowance {
error!(
"Execution value is greater than the allowance. Execution Value: {:?}, Allowance: {:?}",
Expand Down
Loading

0 comments on commit 4ada030

Please sign in to comment.