Skip to content

Conversation

MoMannn
Copy link
Contributor

@MoMannn MoMannn commented Oct 16, 2025

Description

This PR fixes a critical security vulnerability in the Blockaid integration for permission requests. Previously, when validating permission requests through the Gator snap, Blockaid was checking the Gator snap's origin instead of the actual requesting domain found in decodedPermission.origin. This allowed malicious domains to bypass security checks by routing their permission requests through the Gator snap.

The Problem:

  • Permission requests were validated against the Gator snap origin instead of the actual DApp origin
  • Malicious domains could bypass Blockaid security checks by using the Gator snap as a proxy
  • The normalizePPOMRequest function was not extracting the correct origin from decodedPermission.origin

The Solution:

  • Modified normalizePPOMRequest to check for decodedPermission.origin when present
  • Permission requests now use the actual DApp origin for Blockaid validation
  • Maintained backward compatibility for non-permission requests
  • Added comprehensive tests to ensure the fix works correctly

Open in GitHub Codespaces

Changelog

CHANGELOG entry: null

Manual testing steps

  1. Set up a test environment with Gator permissions enabled
  2. Create a permission request with a malicious domain in decodedPermission.origin
  3. Verify that Blockaid now correctly validates against the malicious domain instead of the Gator snap origin
  4. Confirm that regular (non-permission) signature requests continue to work as before
  5. Run the new test suite to verify all scenarios are covered

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Validates typed-sign requests using the permission’s origin when initiated by preinstalled snaps, with tests covering snap/non-snap and fallback cases.

  • PPOM Utils:
    • Use decodedPermission.origin as origin for eth_signTypedData_* when the request comes from a preinstalled snap (isSnapId + isSnapPreinstalled).
    • Pass controllerObject into normalizeSignatureRequest to enable origin override logic; keep typed data sanitization and transaction normalization.
  • Tests:
    • Add mocks for isSnapId and isSnapPreinstalled and new cases validating origin handling:
      • Preinstalled snap uses permission origin.
      • Non-preinstalled snap and non-snap requests do not override origin.
      • Fallback to request origin when no permission present; covers malicious domain scenario.

Written by Cursor Bugbot for commit eca215b. This will update automatically on new commits. Configure here.

@MoMannn MoMannn requested a review from a team as a code owner October 16, 2025 11:53
Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added team-delegation MetaMask Delegation Team INVALID-PR-TEMPLATE PR's body doesn't match template labels Oct 16, 2025
@metamaskbot
Copy link
Collaborator

metamaskbot commented Oct 16, 2025

✨ Files requiring CODEOWNER review ✨

@MetaMask/confirmations (2 files, +389 -2)
  • 📁 app/
    • 📁 scripts/
      • 📁 lib/
        • 📁 ppom/
          • 📄 ppom-util.test.ts +360 -0
          • 📄 ppom-util.ts +29 -2

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: d42040c | Date: 10/16/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.05s (±70ms) 🟡 | historical mean value: 1.05s ⬆️ (historical data)
  • domContentLoaded-> current mean value: 736ms (±68ms) 🟢 | historical mean value: 735ms ⬆️ (historical data)
  • firstContentfulPaint-> current mean value: 80ms (±30ms) 🟢 | historical mean value: 80ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.05s 70ms 1.01s 1.32s 1.26s 1.32s
domContentLoaded 736ms 68ms 696ms 1.01s 939ms 1.01s
firstPaint 80ms 30ms 60ms 348ms 88ms 348ms
firstContentfulPaint 80ms 30ms 60ms 348ms 88ms 348ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [d42040c]
UI Startup Metrics (1237 ± 66 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1237109014456612771328
load106695812365811071157
domContentLoaded106195512315711011142
domInteractive1914137141641
firstPaint633112115942210711137
backgroundConnect2502372858254260
firstReactRender26185682845
getState16566102036
initialActions607710517
loadScripts81670699057852903
setupStore962631019
WebpackHomeuiStartup8257081085698501004
load62557593568628818
domContentLoaded61656891867621811
domInteractive151175101435
firstPaint22555946213246608
backgroundConnect21104372533
firstReactRender27165993136
getState931631113
initialActions3011246
loadScripts61356690765620800
setupStore952331113
FirefoxBrowserifyHomeuiStartup14631259183812315351726
load1230108014058413001373
domContentLoaded1230108014048413001373
domInteractive1023429453102244
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect38211952439101
firstReactRender30254233135
getState94516919
initialActions61638424
loadScripts1205106513828212831343
setupStore146104161161
WebpackHomeuiStartup15991421206412416381871
load1359121016469213901567
domContentLoaded1358120916469213901567
domInteractive1073342775103367
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect382291144370
firstReactRender34268183744
getState164169281074
initialActions42284413
loadScripts1335119416179013671541
setupStore136144191135
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 182 Bytes (0%)
  • ui: 0 Bytes (0%)
  • common: 10 Bytes (0%)

@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: 2b9e33c | Date: 10/16/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.07s (±74ms) 🟡 | historical mean value: 1.05s ⬆️ (historical data)
  • domContentLoaded-> current mean value: 750ms (±70ms) 🟢 | historical mean value: 735ms ⬆️ (historical data)
  • firstContentfulPaint-> current mean value: 79ms (±15ms) 🟢 | historical mean value: 79ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.07s 74ms 1.03s 1.36s 1.29s 1.36s
domContentLoaded 750ms 70ms 712ms 1.02s 951ms 1.02s
firstPaint 79ms 15ms 60ms 208ms 88ms 208ms
firstContentfulPaint 79ms 15ms 60ms 208ms 88ms 208ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [2b9e33c]
UI Startup Metrics (1295 ± 66 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1295116715506613241430
load110798913365711391215
domContentLoaded109895513305711261208
domInteractive19144651831
firstPaint633152133744611141201
backgroundConnect26424734112269286
firstReactRender3118110133549
getState17691101936
initialActions61537714
loadScripts842701106556871937
setupStore1153441219
WebpackHomeuiStartup8237051279838291009
load634572110090637853
domContentLoaded626568108089628842
domInteractive15113671336
firstPaint183561088191169584
backgroundConnect20103562531
firstReactRender25164073132
getState942231113
initialActions3015236
loadScripts624566106986627828
setupStore952231114
FirefoxBrowserifyHomeuiStartup14301254189812714961661
load1208107915128812771351
domContentLoaded1208107915128812761351
domInteractive1063243867103287
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect342292123859
firstReactRender29255043038
getState94416818
initialActions6112414415
loadScripts1186106014278312451328
setupStore136154181035
WebpackHomeuiStartup15341371197211015751775
load1312118815928013611475
domContentLoaded1312118715928013601474
domInteractive93293565495239
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3519125154166
firstReactRender32267883338
getState85596815
initialActions7119020326
loadScripts1291117215687913381446
setupStore135134171148
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 488 Bytes (0.01%)
  • ui: -523 Bytes (-0.01%)
  • common: 10 Bytes (0%)

cursor[bot]

This comment was marked as outdated.

@MoMannn MoMannn requested a review from jeffsmale90 October 17, 2025 11:49
@metamaskbot
Copy link
Collaborator

📊 Page Load Benchmark Results

Current Commit: eca215b | Date: 10/17/2025

📄 Localhost MetaMask Test Dapp

Samples: 100

Summary

  • pageLoadTime-> current mean value: 1.04s (±72ms) 🟡 | historical mean value: 1.05s ⬇️ (historical data)
  • domContentLoaded-> current mean value: 730ms (±69ms) 🟢 | historical mean value: 741ms ⬇️ (historical data)
  • firstContentfulPaint-> current mean value: 77ms (±12ms) 🟢 | historical mean value: 79ms ⬇️ (historical data)
📈 Detailed Results
Metric Mean Std Dev Min Max P95 P99
pageLoadTime 1.04s 72ms 999ms 1.30s 1.27s 1.30s
domContentLoaded 730ms 69ms 697ms 994ms 956ms 994ms
firstPaint 77ms 12ms 60ms 180ms 92ms 180ms
firstContentfulPaint 77ms 12ms 60ms 180ms 92ms 180ms
largestContentfulPaint 0ms 0ms 0ms 0ms 0ms 0ms

Results generated automatically by MetaMask CI

@metamaskbot
Copy link
Collaborator

Builds ready [eca215b]
UI Startup Metrics (1238 ± 55 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1238113614675512731332
load106397812114910941148
domContentLoaded105797112064810901145
domInteractive18145171737
firstPaint71174121542410741126
backgroundConnect2532392797257265
firstReactRender25196362634
getState1654582031
initialActions606910617
loadScripts81073495047843905
setupStore962831013
WebpackHomeuiStartup833717107663852968
load62858391163637811
domContentLoaded62057689961630806
domInteractive15115081337
firstPaint18855877184192662
backgroundConnect21103862632
firstReactRender27175783235
getState931731114
initialActions309247
loadScripts61757488959628795
setupStore1061521213
FirefoxBrowserifyHomeuiStartup14771250186710915291664
load1249108414797513071375
domContentLoaded1249108314797513071374
domInteractive1163434355123249
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect3823139194065
firstReactRender27215872744
getState1048810915
initialActions5216716411
loadScripts1223106514557612831355
setupStore146207231134
WebpackHomeuiStartup15611393222713916001874
load1342119417319513961519
domContentLoaded1342119317319513951519
domInteractive1133137877109344
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect362084144071
firstReactRender27227572837
getState83374814
initialActions40719313
loadScripts1319117516839213761494
setupStore125139191033
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 26.05 KiB (0.58%)
  • ui: 11.5 KiB (0.18%)
  • common: 4.78 KiB (0.06%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template size-M team-delegation MetaMask Delegation Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants