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

Ensure transaction parameters from dapps are valid hex #11780

Closed
danjm opened this issue Aug 5, 2021 · 5 comments
Closed

Ensure transaction parameters from dapps are valid hex #11780

danjm opened this issue Aug 5, 2021 · 5 comments
Assignees
Labels
area-api PS-team PS MM extension team issues Sev2-normal Normal severity; minor loss of service or inconvenience. stale issues and PRs marked as stale team-confirmations-planning (only for internal use within Confirmations team) type-bug
Milestone

Comments

@danjm
Copy link
Contributor

danjm commented Aug 5, 2021

See metamask-extension/app/scripts/controllers/transactions/lib/util.js

@darkwing darkwing added the Sev2-normal Normal severity; minor loss of service or inconvenience. label Aug 10, 2021
@danjm danjm added the area-api label Sep 24, 2021
@danjm danjm added this to the Q4-Bug-Bash milestone Oct 4, 2021
@EHaracic EHaracic self-assigned this Oct 19, 2021
@david0xd
Copy link
Contributor

Hi @danjm, can you please provide more information on this?
Are we verifying here the custom user input from the UI or the whole transaction parameter data internally?
I'm considering this function as the target to validate the HEX: validateTxParams(txParams, eip1559Compatibility = true).
I'm interested from where I can trigger the action (test-dapp) so we can later see the bad parameter inputs?
If you can provide a screenshot if this can be validated from the UI, it will also be very useful.

@danjm
Copy link
Contributor Author

danjm commented Oct 22, 2021

validateTxParams is the correct place to add the validation

An invalid transaction will have to be created from the test dapp. One way to test is you could build the test dapp locally and directly edit the code for sending transactions:

https://github.com/MetaMask/test-dapp/blob/main/src/index.js#L321

It will be easier to test this once this task is resolved MetaMask/test-dapp#128

@dragana8 dragana8 assigned dragana8 and unassigned EHaracic Nov 3, 2021
@ghost ghost assigned ghost and dragana8 and unassigned dragana8 and ghost Nov 5, 2021
@EHaracic EHaracic added the PS-team PS MM extension team issues label Jan 21, 2022
@hilvmason hilvmason added stability team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead and removed stability labels Apr 22, 2022
@vandan
Copy link

vandan commented Jun 14, 2022

@hilvmason Can you confirm if any support is needed from the dapp API team to close this issue?
If not, can you remove the "area-api" label? Thank you!

@hilvmason hilvmason added team-confirmations-planning (only for internal use within Confirmations team) and removed team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead labels Jun 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2023

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.

@github-actions github-actions bot added the stale issues and PRs marked as stale label Sep 3, 2023
@github-actions
Copy link
Contributor

This issue was closed because there has been no follow up activity in the last 45 days. If you feel this was closed in error, please reopen and provide evidence on the latest release of the extension. Thank you for your contributions.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-api PS-team PS MM extension team issues Sev2-normal Normal severity; minor loss of service or inconvenience. stale issues and PRs marked as stale team-confirmations-planning (only for internal use within Confirmations team) type-bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants