Skip to content

Commit ee0d4b2

Browse files
committed
Fix owner check text and signer check example
1 parent 5225483 commit ee0d4b2

File tree

2 files changed

+7
-12
lines changed

2 files changed

+7
-12
lines changed

Diff for: not-so-smart-contracts/solana/ownership_check/README.md

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ Accounts in Solana include metadata of an owner. These owners are identified by
44
This malicious account will inherently have a different program ID as owner, but considering there’s no check that the program ID is the same, as long as the other preconditions are passed, the attacker can trick the program into thinking their malicious account is the expected account.
55

66
## Exploit Scenario
7-
The following contract updates the current market authority with a new one. Unfortunately, the only check being done here is against the current authority’s public key prior to setting a new authority.
8-
Therefore, a malicious actor can set themselves as the new authority without being the actual market authority. This is because the ownership of the market authority account isn’t being fully verified against itself by program ID. Since there’s no check that the market is owned by the program itself, an attacker can pass in their own fabricated account with spoofed data which is then verified against the public key of the current authority’s account, making it easy for the attacker to set themselves as the new authority.
7+
The following contract allows funds to be dispersed from an escrow account vault, provided the escrow account's state is `Complete`. Unfortunately, there is no check that the `State` account is owned by the program.
8+
Therefore, a malicious actor can pass in their own fabricated `State` account with spoofed data, allowing the attacker to send the vault's funds to themselves.
99

1010
### Example Contract
1111
```rust

Diff for: not-so-smart-contracts/solana/signer_check/README.md

+5-10
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,18 @@ In Solana, each public key has an associated private key that can be used to gen
44
In case certain permissions are required to perform a sensitive function of the contract, a missing signer check becomes an issue. Without this check, an attacker would be able to call the respective access controlled functions permissionlessly.
55

66
## Exploit Scenario
7-
The following contract updates the current market authority with a new one. Unfortunately, the only check being done here is against the current authority’s public key prior to setting a new authority.
8-
Therefore, a malicious actor can set themselves as the new authority without being the actual market authority. This is because the current authority’s private key isn’t being verified considering the contract doesn’t check whether the account holder has signed or not.
7+
The following contract sets an escrow account's state to `Complete`. Unfortunately, the contract does not check whether the escrow account holder has signed the transaction.
8+
Therefore, a malicious actor can set the state to `Complete`, without needing access to the escrow account holders’s private key.
99

1010
### Example Contract
1111
```rust
1212
fn pay_escrow(_program_id: &Pubkey, accounts: &[AccountInfo], _instruction_data: &[u8]) -> ProgramResult {
1313
let account_info_iter = &mut accounts.iter();
1414
let state_info = next_account_info(account_info_iter)?;
15-
let escrow_vault_info = next_account_info(account_info_iter)?;
16-
let escrow_receiver_info = next_account_info(account_info_iter)?;
1715

18-
let state = State::deserialize(&mut &**state_info.data.borrow())?;
19-
20-
if state.escrow_state == EscrowState::Complete {
21-
**escrow_vault_info.try_borrow_mut_lamports()? -= state.amount;
22-
**escrow_receiver_info.try_borrow_mut_lamports()? += state.amount;
23-
}
16+
let mut state = State::deserialize(&mut &**state_info.data.borrow())?;
17+
state.escrow_state = EscrowState::Complete;
18+
state.serialize(&mut &mut **state_info.data.borrow_mut())?;
2419

2520
Ok(())
2621
}

0 commit comments

Comments
 (0)