Audit Fixes 8 : Silent failures when unregistering non-existent sub-accounts (#552)

Author: Kayanski

framework/contracts/account/src/error.rs

@@ -81,8 +81,11 @@ pub enum AccountError {
     NotWhitelisted {},
 
     // ** Sub Account ** //
-    #[error("Removing sub account failed")]
-    SubAccountRemovalFailed {},
+    #[error("Sub account doesn't exist")]
+    SubAccountDoesntExist {},
+
+    #[error("Sub account is expected to be the caller")]
+    SubAccountIsNotCaller {},
 
     #[error("Register of sub account failed")]
     SubAccountRegisterFailed {},

framework/contracts/account/src/sub_account.rs

@@ -113,15 +113,19 @@ fn unregister_sub_account(deps: DepsMut, env: &Env, info: MessageInfo, id: u32)
         &AccountId::local(id),
     )?;
 
-    if account.is_some_and(|a| a.addr() == info.sender) {
-        SUB_ACCOUNTS.remove(deps.storage, id);
-
-        Ok(AccountResponse::new(
-            "unregister_sub_account",
-            vec![("sub_account_removed", id.to_string())],
-        ))
+    if let Some(account) = account {
+        if account.addr() == info.sender {
+            SUB_ACCOUNTS.remove(deps.storage, id);
+
+            Ok(AccountResponse::new(
+                "unregister_sub_account",
+                vec![("sub_account_removed", id.to_string())],
+            ))
+        } else {
+            Err(AccountError::SubAccountIsNotCaller {})
+        }
     } else {
-        Err(AccountError::SubAccountRemovalFailed {})
+        Err(AccountError::SubAccountDoesntExist {})
     }
 }