Skip to content

Commit

Permalink
fix: L-03 Missing Checks in ERC-20 Implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
qalisander committed Oct 4, 2024
1 parent e502d12 commit 1c61be6
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
2 changes: 1 addition & 1 deletion contracts/src/token/erc20/extensions/permit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ impl<T: IEip712 + StorageType> Erc20Permit<T> {
return Err(ERC2612InvalidSigner { signer, owner }.into());
}

self.erc20._approve(owner, spender, value)?;
self.erc20._approve(owner, spender, value, true)?;

Ok(())
}
Expand Down
33 changes: 26 additions & 7 deletions contracts/src/token/erc20/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ sol! {
#[allow(missing_docs)]
error ERC20InvalidSpender(address spender);

/// Indicates a failure with the `approver` of a token to be approved. Used in approvals.
/// approver Address initiating an approval operation.
///
/// * `approver` - Address initiating an approval operation.
#[derive(Debug)]
#[allow(missing_docs)]
error ERC20InvalidApprover(address approver);

}

/// An [`Erc20`] error defined as described in [ERC-6093].
Expand All @@ -90,6 +98,9 @@ pub enum Error {
/// Indicates a failure with the `spender` to be approved. Used in
/// approvals.
InvalidSpender(ERC20InvalidSpender),
/// Indicates a failure with the `approver` of a token to be approved. Used
/// in approvals. approver Address initiating an approval operation.
InvalidApprover(ERC20InvalidApprover),
}

impl MethodError for Error {
Expand Down Expand Up @@ -270,7 +281,7 @@ impl IErc20 for Erc20 {
value: U256,
) -> Result<bool, Self::Error> {
let owner = msg::sender();
self._approve(owner, spender, value)
self._approve(owner, spender, value, true)
}

fn transfer_from(
Expand All @@ -296,6 +307,7 @@ impl Erc20 {
/// * `&mut self` - Write access to the contract's state.
/// * `owner` - Account that owns the tokens.
/// * `spender` - Account that will spend the tokens.
/// * `emit_event` - Emit an [`Approval`] event flag.
///
/// # Errors
///
Expand All @@ -310,15 +322,24 @@ impl Erc20 {
owner: Address,
spender: Address,
value: U256,
emit_event: bool,
) -> Result<bool, Error> {
if owner.is_zero() {
return Err(Error::InvalidApprover(ERC20InvalidApprover {
approver: Address::ZERO,
}));
}

if spender.is_zero() {
return Err(Error::InvalidSpender(ERC20InvalidSpender {
spender: Address::ZERO,
}));
}

self._allowances.setter(owner).insert(spender, value);
evm::log(Approval { owner, spender, value });
if emit_event {
evm::log(Approval { owner, spender, value });
}
Ok(true)
}

Expand Down Expand Up @@ -523,7 +544,7 @@ impl Erc20 {
spender: Address,
value: U256,
) -> Result<(), Error> {
let current_allowance = self._allowances.get(owner).get(spender);
let current_allowance = self.allowance(owner, spender);
if current_allowance != U256::MAX {
if current_allowance < value {
return Err(Error::InsufficientAllowance(
Expand All @@ -535,9 +556,7 @@ impl Erc20 {
));
}

self._allowances
.setter(owner)
.insert(spender, current_allowance - value);
self._approve(owner, spender, current_allowance - value, false)?;
}

Ok(())
Expand Down Expand Up @@ -825,7 +844,7 @@ mod tests {
.setter(msg::sender())
.set(one);
let result = contract.transfer_from(Address::ZERO, alice, one);
assert!(matches!(result, Err(Error::InvalidSender(_))));
assert!(matches!(result, Err(Error::InvalidApprover(_))));
}

#[motsu::test]
Expand Down

0 comments on commit 1c61be6

Please sign in to comment.