code-423n4 / 2022-11-redactedcartel-findings

3 stars 2 forks source link

Possible funds lost in `AutoPxGlp` due to missing address check #279

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L449-L456 https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L436-L444

Vulnerability details

Impact

AutoPxGlp::redeem and AutoPxGlp::withdraw does not check if receiver address is not zero. And always it's receiver, who gets the assets. Not checking it may lead to funds lost permemently.

Proof of Concept

  1. User calls impacted function iwth default receiver (address 0)
  2. All funds are sent to address 0, hence lost

Tools Used

VS Code

Recommended Mitigation Steps

Add check for 0 address, and for 0 amount, to save gas

diff --git a/src/vaults/AutoPxGlp.sol b/src/vaults/AutoPxGlp.sol
index 15253e0..f7c9aa4 100644
--- a/src/vaults/AutoPxGlp.sol
+++ b/src/vaults/AutoPxGlp.sol
@@ -438,6 +438,9 @@ contract AutoPxGlp is PirexERC4626, PxGmxReward, ReentrancyGuard {
         address receiver,
         address owner
     ) public override returns (uint256 shares) {
+        if (receiver == address(0)) revert ZeroAddress();
+        if (assets == 0) revert ZeroAddress();
+
         compound(1, 1, true);

         shares = PirexERC4626.withdraw(assets, receiver, owner);
@@ -451,6 +454,9 @@ contract AutoPxGlp is PirexERC4626, PxGmxReward, ReentrancyGuard {
         address receiver,
         address owner
     ) public override returns (uint256 assets) {
+        if (receiver == address(0)) revert ZeroAddress();
+        if (shares == 0) revert ZeroAddress();
+
         compound(1, 1, true);

         assets = PirexERC4626.redeem(shares, receiver, owner);
c4-judge commented 1 year ago

Duplicate of https://github.com/code-423n4/2022-11-redactedcartel-findings/issues/282