Validate pubkey, relay URL and secret of NWC URL (#810)

* Validate pubkey, relay URL and secret of NWC URL

* Fix NWC secret regexp

* Use sequential validation in Yup schema

* Add note about possible mismatch between hostnames and pubkeys

* Remove unused param

components/webln/nwc.js

@@ -2,6 +2,7 @@
 
 import { createContext, useCallback, useContext, useEffect, useState } from 'react'
 import { Relay, finalizeEvent, nip04 } from 'nostr-tools'
+import { parseNwcUrl } from '../../lib/url'
 
 const NWCContext = createContext()
 
@@ -30,7 +31,7 @@ export function NWCProvider ({ children }) {
     const { nwcUrl } = config
     setNwcUrl(nwcUrl)
 
-    const params = parseWalletConnectUrl(nwcUrl)
+    const params = parseNwcUrl(nwcUrl)
     setRelayUrl(params.relayUrl)
     setWalletPubkey(params.walletPubkey)
     setSecret(params.secret)
@@ -56,7 +57,7 @@ export function NWCProvider ({ children }) {
       return
     }
 
-    const params = parseWalletConnectUrl(nwcUrl)
+    const params = parseNwcUrl(nwcUrl)
     setRelayUrl(params.relayUrl)
     setWalletPubkey(params.walletPubkey)
     setSecret(params.secret)
@@ -229,22 +230,3 @@ async function getInfoWithRelay (relay, walletPubkey) {
     })
   })
 }
-
-function parseWalletConnectUrl (walletConnectUrl) {
-  walletConnectUrl = walletConnectUrl
-    .replace('nostrwalletconnect://', 'http://')
-    .replace('nostr+walletconnect://', 'http://') // makes it possible to parse with URL in the different environments (browser/node/...)
-
-  const url = new URL(walletConnectUrl)
-  const params = {}
-  params.walletPubkey = url.host
-  const secret = url.searchParams.get('secret')
-  const relayUrl = url.searchParams.get('relay')
-  if (secret) {
-    params.secret = secret
-  }
-  if (relayUrl) {
-    params.relayUrl = relayUrl
-  }
-  return params
-}

lib/url.js

@@ -27,6 +27,32 @@ export function stripTrailingSlash (uri) {
   return uri.endsWith('/') ? uri.slice(0, -1) : uri
 }
 
+export function parseNwcUrl (walletConnectUrl) {
+  if (!walletConnectUrl) return {}
+
+  walletConnectUrl = walletConnectUrl
+    .replace('nostrwalletconnect://', 'http://')
+    .replace('nostr+walletconnect://', 'http://') // makes it possible to parse with URL in the different environments (browser/node/...)
+
+  // XXX There is a bug in parsing since we use the URL constructor for parsing:
+  // A wallet pubkey matching /^[0-9a-fA-F]{64}$/ might not be a valid hostname.
+  // Example: 11111111111 (10 1's) is a valid hostname (gets parsed as IPv4) but 111111111111 (11 1's) is not.
+  // See https://stackoverflow.com/questions/56804936/how-does-only-numbers-in-url-resolve-to-a-domain
+  // However, this seems to only get triggered if a wallet pubkey only contains digits so this is pretty improbable.
+  const url = new URL(walletConnectUrl)
+  const params = {}
+  params.walletPubkey = url.host
+  const secret = url.searchParams.get('secret')
+  const relayUrl = url.searchParams.get('relay')
+  if (secret) {
+    params.secret = secret
+  }
+  if (relayUrl) {
+    params.relayUrl = relayUrl
+  }
+  return params
+}
+
 // eslint-disable-next-line
 export const URL_REGEXP = /^((https?|ftp):\/\/)?(www.)?(((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:)*@)?(((\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5])\.(\d|[1-9]\d|1\d\d|2[0-4]\d|25[0-5]))|((([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|\d|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.)+(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])*([a-z]|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])))\.?)(:\d*)?)(\/((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)+(\/(([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)*)*)?)?(\?((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|[\uE000-\uF8FF]|\/|\?)*)?(\#((([a-z]|\d|-|\.|_|~|[\u00A0-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF])|(%[\da-f]{2})|[!\$&'\(\)\*\+,;=]|:|@)|\/|\?)*)?$/i
 

lib/validate.js

@@ -10,6 +10,8 @@ import { msatsToSats, numWithUnits, abbrNum } from './format'
 import * as usersFragments from '../fragments/users'
 import * as subsFragments from '../fragments/subs'
 import { B64_REGEX, HEX_REGEX, isInvoicableMacaroon, isInvoiceMacaroon } from './macaroon'
+import { parseNwcUrl } from './url'
+
 const { SUB } = subsFragments
 const { NAME_QUERY } = usersFragments
 
@@ -115,6 +117,20 @@ addMethod(string, 'https', function () {
   })
 })
 
+addMethod(string, 'wss', function (msg) {
+  return this.test({
+    name: 'wss',
+    message: msg || 'wss required',
+    test: (url) => {
+      try {
+        return new URL(url).protocol === 'wss:'
+      } catch {
+        return false
+      }
+    }
+  })
+})
+
 const titleValidator = string().required('required').trim().max(
   MAX_TITLE_LENGTH,
   ({ max, value }) => `-${Math.abs(max - value.length)} characters remaining`
@@ -531,7 +547,29 @@ export const lnbitsSchema = object({
 })
 
 export const nwcSchema = object({
-  nwcUrl: string().required('required').trim().matches(/^nostr\+walletconnect:/)
+  nwcUrl: string()
+    .required('required')
+    .test(async (nwcUrl, context) => {
+      // run validation in sequence to control order of errors
+      // inspired by https://github.com/jquense/yup/issues/851#issuecomment-1049705180
+      try {
+        await string().required('required').validate(nwcUrl)
+        await string().matches(/^nostr\+?walletconnect:\/\//, { message: 'must start with nostr+walletconnect://' }).validate(nwcUrl)
+        let relayUrl, walletPubkey, secret
+        try {
+          ({ relayUrl, walletPubkey, secret } = parseNwcUrl(nwcUrl))
+        } catch {
+          // invalid URL error. handle as if pubkey validation failed to not confuse user.
+          throw new Error('pubkey must be 64 hex chars')
+        }
+        await string().required('pubkey required').trim().matches(NOSTR_PUBKEY_HEX, 'pubkey must be 64 hex chars').validate(walletPubkey)
+        await string().required('relay url required').trim().wss('relay must use wss://').validate(relayUrl)
+        await string().required('secret required').trim().matches(/^[0-9a-fA-F]{64}$/, 'secret must be 64 hex chars').validate(secret)
+      } catch (err) {
+        return context.createError({ message: err.message })
+      }
+      return true
+    })
 })
 
 export const bioSchema = object({