Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Certain PPOM signatures are not functioning as expected. #27905

Closed
sleepytanya opened this issue Oct 16, 2024 · 7 comments
Closed

[Bug]: Certain PPOM signatures are not functioning as expected. #27905

sleepytanya opened this issue Oct 16, 2024 · 7 comments
Assignees
Labels
regression-develop Regression bug that was found on development branch, but not yet present in production Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team type-bug

Comments

@sleepytanya
Copy link
Contributor

sleepytanya commented Oct 16, 2024

Describe the bug

Malicious ERC 20 Approval (BUSD), Malicious Set Approval for All, and Malicious ERC20 Approval with Odd Hex Data are not being identified by PPOM.
Not sure if that's expected but the token name on BNB is displayed as anyUSDC and in RC 12.5.0 Malicious ERC 20 Approval (BUSD) is not flagged (account has enough USDC).

Expected behavior

Screenshots/Recordings

mainnet1 mainnet2 mainnet3 bnb1 bnb2 Screenshot 2024-10-16 at 09 35 23

Steps to reproduce

  1. Connect to test dapp
  2. Click Malicious ERC 20 Approval (BUSD), Malicious Set Approval for All, or Malicious ERC20 Approval with Odd Hex Data

Error messages or log output

No response

Detection stage

In production (default)

Version

develop

Build type

None

Browser

Chrome

Operating system

MacOS

Hardware wallet

No response

Additional context

No response

Severity

No response

@metamaskbot metamaskbot added regression-prod-12.5.0 Regression bug that was found in production in release 12.5.0 and removed regression-RC-12.5.0 labels Oct 16, 2024
@sleepytanya sleepytanya added regression-develop Regression bug that was found on development branch, but not yet present in production regression-RC-12.5.0 and removed regression-prod-12.5.0 Regression bug that was found in production in release 12.5.0 labels Oct 16, 2024
@sleepytanya sleepytanya reopened this Oct 16, 2024
@sleepytanya
Copy link
Contributor Author

Latest develop build

Malicious ERC 20 Transfer(USDC) is not flagged on - Ethereum, Linea, BNB
Malicious ERC 20 Approval (BUSD) is not flagged on - Ethereum, Linea
Malicious Set Approval for All is not flagged on - Ethereum, Linea, Avalanche
Malicious Permit is not flagged on - zkSync
Malicious Seaport is not flagged on - zkSync
Set ETH Malicious x10 Batch is not flagged on - zkSync
Set ETH Malicious x10 Queue is not flagged on - zkSync
Malicious Approval with Odd Hex Data is not flagged on - Linea
Malicious Permit with Padded ChainID is not flagged on - zkSync
Sign Permit is not flagged on - Linea, Avalanche, zkSync

ERC20 transfer on BNB (header):

Screenshot 2024-10-18 at 13 23 35

@Unik0rnMaggie
Copy link
Contributor

Unik0rnMaggie commented Oct 21, 2024

On the latest develop build, getting the same above results, except for Ethereum Mainnet and zkSync:

  1. Ethereum Mainnet:

Console error Error validating JSON RPC using PPOM: Error: simulation: fallback: (code: -32000, message: , data: None) for all the below:

      Send EIP 1559 Transaction
      Send EIP 1559 without gas
      Malicious ERC20 transfer (USDC) 
      Malicious ERC20 Approval (BUSD)
      Malicious Set Approval for All 
      Malicious ERC 20 Approval with Odd Hex Data 
2. zkSync:

No console error and only the following are not flagged:

Malicious Permit 
Malicious Seaport
Malicious Permit with Padded ChainID 
Ethereum.Mainnet.Chrome.mov
zksyn.chrome.mov

@bschorchit
Copy link

bschorchit commented Oct 21, 2024

The errors should recently have been fixed with this PR: #27939. As @Unik0rnMaggie reported they are not, we should investigate what might be causing failures on Ethereum cc: @jpuri .

It not behaving correctly on other networks is likely because the current address used on those request is not a valid token/nft on those networks.

@sleepytanya
Copy link
Contributor Author

sleepytanya commented Oct 22, 2024

@Unik0rnMaggie
Latest develop.
Ethereum:

EIP 1559 and Legacy - same error in the console Error validating JSON RPC using PPOM: Error: simulation: fallback: (code: -32000, message: , data: None), yellow warning:

Screenshot 2024-10-21 at 23 07 56

Not flagged:

 Malicious ERC20 transfer (USDC) 
 Malicious ERC20 Approval (BUSD)
 Malicious Set Approval for All 
 Malicious ERC 20 Approval with Odd Hex Data

@bschorchit bschorchit added Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. and removed regression-RC-12.5.0 labels Oct 22, 2024
@jpuri
Copy link
Contributor

jpuri commented Oct 25, 2024

I checked with latest code in develop branch today and all malicious transactions except Malicious ERC20 Transfer (USDC) is working on mainnet. This one I think was not working even before.

@Unik0rnMaggie
Copy link
Contributor

Hi Team,

As per chat with @jpuri it seems the errors might occur due to missing access on provider for my Infura Key.

With the production Infura key I do not experience any errors.

@bschorchit would it be possible to receive the access on my Infura key to verify if the error persists?

Thank you

@bschorchit
Copy link

Closing as this is not an issue with the production Infura key! We can re-open if we encounter this again.

@Unik0rnMaggie I'll follow up with you re: permissions for Infura key on slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression-develop Regression bug that was found on development branch, but not yet present in production Sev1-high High severity; partial loss of service with severe impact upon users, with no workaround. team-confirmations Push issues to confirmations team type-bug
Projects
Archived in project
Status: Fixed
Development

No branches or pull requests

5 participants