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

Patch external_url method to constrain set of valid uris #1000

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

rkachowski
Copy link

Description

Patches the external_url method in apps/block_scout_web/lib/block_scout_web/views/tokens/instance/overview_view.ex to only consider urls that begin with http as valid. This allows both "http" and "https" external urls to be rendered within the page and treats any other uri as missing.

This is needed to prevent a reported XSS exploit whereby an NFT encoded a javascript:eval('blah') within the external_url content of it's own metadata.

Other changes

Minor whitespace changes

Tested

  • Tested locally against alfajores db
  • Developed unit test for this case
  • Passed test suite

Issues

@rkachowski rkachowski changed the title Dhutch/xss Patch external_url method to constrain set of valid uris Dec 7, 2023
@rkachowski rkachowski marked this pull request as ready for review December 7, 2023 10:40
@rkachowski rkachowski requested a review from a team as a code owner December 7, 2023 10:40
@rkachowski rkachowski requested a review from lvpeschke December 7, 2023 10:40
Copy link

github-actions bot commented Dec 7, 2023

Unit Test Results

       5 files  +  1     363 suites  +82   3m 35s ⏱️ -1s
2 637 tests +84  2 563 ✔️ +84  74 💤 ±0  0 ±0 
2 651 runs  +84  2 575 ✔️ +84  76 💤 ±0  0 ±0 

Results for commit 83daba8. ± Comparison against base commit eec0e90.

@rkachowski rkachowski enabled auto-merge (squash) December 7, 2023 11:05
@rkachowski rkachowski merged commit 1b000be into master Dec 7, 2023
19 checks passed
@rkachowski rkachowski deleted the dhutch/xss branch December 7, 2023 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants