The-Poolz / Poolz-Back

smart contracts using solidity for erc20 tokens to eth and erc20 to erc20
https://www.poolz.finance
MIT License
14 stars 9 forks source link

Bug : Able to confirm Metamask transaction (Spend limit) for past pool date/time #113

Open crypto-x-soldier opened 3 years ago

crypto-x-soldier commented 3 years ago

Describe the bug I am able to set Pool joining date/time to past date by modifying my PC's current date/time. And I was able to approve the transaction in Metamask for Spend Limit

To Reproduce Steps to reproduce the behavior:

  1. Change the PC's current date time to past date. Example Jan 1 2021.
  2. Go to https://app.poolz.finance/dashboard
  3. Click on Tomochain
  4. Click Connect Wallet.
  5. Click Create Pool and enter the following details
    1. Token Contract Address : 0x127205f3A8884bF8B0956c4610e997E6DC0bFad1
    2. To Token : Tomo
  6. Click Set Swipe Ratio
  7. Enter 'Swap Ratio' as 50 and 'Amount of tokens to lock' as 5000
  8. Select the date to Jan 2 2021. Leave the time as it is.
  9. Click Confirm.
  10. Approve the Metamask transaction.
  11. Verify the transaction as 'Success' in the blockchain explorer https://scan.testnet.tomochain.com

Expected behavior In the '2.Swap Rules' section -> Error to be displayed as 'Past Date time ' before navigating to '3.Confirm' section.

Screenshots

https://user-images.githubusercontent.com/77062578/103962575-4d070680-517d-11eb-883e-ee0134e9545c.mp4

Desktop (please complete the following information):

Additional context

  1. The system relies on client side PC to validate the date and time. The logic should be updated to use a server date time for validation.
  2. To double check smart contract so all the important transactions have date/time validated.
  3. One good thing is -> Smart Contract catches the past date/time in the final step of the process. It doesn't allow to create the pool. I think the date/time check is added as part of the fix suggested from the audit report. But still by the above steps still loss of funds (transaction fees) happens for the user.

Wallet address 0xdD5F5feE4Db473ACa5A0b52EE459Abd35D098BEE

crypto-x-soldier commented 3 years ago

The 'Join Pool' also has same issue where it checks the date/time by taking it from client's PC. So when PC's date time is updated to past date the expired pools are shown as available to join

https://user-images.githubusercontent.com/77062578/103963255-cbb07380-517e-11eb-8c0e-cc9a2624bd1e.mp4

Lomet commented 3 years ago

This is very nice. the contract working on the blockchain timestamp, so it will not do the action for real.

crypto-x-soldier commented 3 years ago

In the createpool video I have shown the transaction hash 'Success' in explorer. Pls check.

Also it is advisable to have the system date time checks on the server side or atleast you could use a JS client side library which in turn talks to server to get the date/time and use that for validation. Depending on client's device's date/time is risky and it might lead to unforeseen hacks in the future

Lomet commented 3 years ago

the 'Success' is for the approved spending, not for the real action.

we will add a message that tells that the local time is wrong, (if it wrong)

very nice point!