Skip to content

fix: payment signature format validation and SIWX ETS size cap#39

Merged
cardotrejos merged 8 commits into
mainfrom
fix/payment-signature-validation-and-siwx-ets-cap
Mar 28, 2026
Merged

fix: payment signature format validation and SIWX ETS size cap#39
cardotrejos merged 8 commits into
mainfrom
fix/payment-signature-validation-and-siwx-ets-cap

Conversation

@cardotrejos

@cardotrejos cardotrejos commented Mar 28, 2026

Copy link
Copy Markdown
Owner

Security Council Findings — March 28, 2026

🟡 HIGH: PaymentSignature.validate only checks field presence, not format

Problem: validate/2 checked that transactionHash and payerWallet were present and non-empty strings, but never validated their format. Arbitrary strings like "fake" or "AAAA" pass structural validation — a spoofed payment proof could slip through if the facilitator is misconfigured, offline, or not performing independent verification.

Fix: Added validate_field_formats/1 called after missing-field check:

  • transactionHash: must match 0x + 64 hex chars (EVM) OR base58 87–88 chars (Solana tx signature)
  • payerWallet: must match 0x + 40 hex chars (EVM) OR base58 32–44 chars (Solana address)

Returns {:error, {:invalid_format, [{field, :invalid_format}]}} on failure with telemetry emitted.

network and scheme are intentionally NOT validated here — those are validated downstream by facilitator/scheme validators which have the authoritative allowlists.


🟡 HIGH: SIWX ETSStorage has no max_size cap

Problem: Unlike PaymentIdentifier.ETSCache, SIWX.ETSStorage had no size limit. An attacker generating unique {address, resource} pairs would grow the ETS table without bound, eventually OOMing the node.

Fix: Added max_size option (default: 100,000 entries). When the table is at capacity:

  • New insertions return {:error, :storage_full}
  • Updates to existing keys are never rejected (size doesn't grow)
  • Configurable via start_link opts for operator tuning

After the next cleanup sweep evicts expired records, new entries can be accepted again.


🟢 LOW: :rand.uniform/1 backoff seeding

Assessment: Acceptable. OS process entropy is sufficient for retry jitter spread — cryptographic PRNG seeding is not needed here. No change.


Note

Medium Risk
Tightens validation for payment proofs and changes SIWX storage behavior to reject writes when the ETS table is full, which could impact integrations that relied on permissive inputs or unlimited growth.

Overview
Hardens PAYMENT-SIGNATURE verification by adding structural format checks for transactionHash (EVM 0x+64 hex or Solana base58 87–88 chars) and payerWallet (EVM 0x+40 hex or Solana base58) in X402.PaymentSignature.validate/2, returning {:error, {:invalid_format, ...}} and emitting telemetry on failures.

Adds a size cap to SIWX ETS storage (X402.Extensions.SIWX.ETSStorage) via a new max_size option (default 100_000); once reached, new {address, resource} inserts return {:error, :storage_full} while updates to existing keys continue to succeed.

Tests and doctests are updated to use realistic 32-byte transaction hashes and to reflect the new validation expectations.

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

Security Council findings — Mar 28, 2026 (x402 SDK scan):

[HIGH] PaymentSignature.validate/2 only checked field presence, not format.
transactionHash and payerWallet accepted arbitrary strings, meaning a
spoofed payment proof (garbage values) would pass structural validation.

Fix: add validate_field_formats/1 called after missing field check:
- transactionHash: must match 0x+64hex (EVM) OR base58 87-88 chars (Solana)
- payerWallet: must match 0x+40hex (EVM) OR base58 32-44 chars (Solana)
Returns {:error, {:invalid_format, [{field, :invalid_format}]}} on failure.
Telemetry emitted for observability. network/scheme validated downstream
by facilitator/scheme validators which have the authoritative allowlists.

[HIGH] SIWX ETSStorage had no max_size cap. A spam attack generating
unique {address, resource} pairs caused unbounded ETS memory growth.

Fix: add max_size option (default: 100_000 entries). New put/4 calls
return {:error, :storage_full} when table is at capacity AND the key
does not already exist (updates are never rejected). Configurable via
start_link opts so operators can tune for their deployment size.

[LOW] :rand.uniform/1 backoff seeding: cryptographic seeding not
needed for jitter — OS process entropy is sufficient for retry spread.
No change needed.
@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@greptile-apps

greptile-apps Bot commented Mar 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR tightens two security-related hardening measures: it adds format validation to X402.PaymentSignature.validate/2 so that structurally invalid hashes and wallet addresses are rejected early, and it caps the SIWX ETSStorage table at a configurable max_size to prevent OOM under spam attacks.

Key changes:

  • PaymentSignature.validate/2 is refactored into a with pipeline that now calls check_field_formats/1 after the missing-fields check; new private helpers validate_transaction_hash/1 and validate_payer_wallet/1 accept EVM (0x + 64/40 hex) or Solana (base58 87–88 / 43–44 chars) formats.
  • ETSStorage gains a max_size NimbleOptions field (default 100_000); handle_call({:put, …}) checks :ets.info(:size) and :ets.member/2 before inserting, returning {:error, :storage_full} only for genuinely new keys that would exceed the cap.
  • All doctests and test fixtures updated to use a properly formatted 64-hex EVM transaction hash.
  • The @solana_address_regex comment incorrectly documents the character range as "32-44" when the regex enforces {43,44} (the correct range for a 32-byte base58-encoded Solana key).
  • The new format-validation code paths and the max_size cap have no unit test coverage, which conflicts with the project's stated >90% coverage target in CLAUDE.md.

Confidence Score: 5/5

Safe to merge; all findings are P2 documentation and test-coverage gaps with no logic defects.

Both new features (format validation and ETS size cap) are logically correct. The GenServer serialisation means there is no TOCTOU race in the size-cap check. The regex values are correct even though one comment mislabels the range. All remaining findings are P2: a one-line comment fix and missing test cases.

test/x402/payment_signature_test.exs and test/x402/extensions/siwx/ets_storage_test.exs are missing coverage for the new validation and size-cap paths.

Important Files Changed

Filename Overview
lib/x402/payment_signature.ex Adds format validation for transactionHash and payerWallet via a new validate_field_formats/1 step; refactors validate/2 to a with pipeline; regex comment for Solana address range is inaccurate (says 32-44, regex is 43-44).
lib/x402/extensions/siwx/ets_storage.ex Adds a configurable max_size option (default 100,000) to prevent unbounded ETS growth; implementation is correct given GenServer serialisation eliminates TOCTOU concerns.
test/x402/payment_signature_test.exs Updated existing tests to use valid-format transaction hashes; no new tests added for the {:invalid_format, ...} error path or Solana format coverage.
test/x402/extensions/siwx/ets_storage_test.exs No changes made to this file; the new max_size cap is entirely untested.
test/x402/plug/payment_gate_test.exs Updated test fixtures and assertion helpers to use a valid 64-hex-char EVM transaction hash; no logic changes.
lib/x402.ex Updated doctests to use a properly formatted EVM transaction hash so they pass with the new format validation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["validate/2 called"] --> B{"is_map payload\nand requirements?"}
    B -- No --> C["{:error, :invalid_payload}"]
    B -- Yes --> D["check_missing_fields/1"]
    D -- "missing fields" --> E["{:error, {:missing_fields, [...]}}"]
    D -- ":ok" --> F["check_field_formats/1"]
    F --> G["validate_transaction_hash/1\n0x+64hex OR Solana base58 87-88"]
    F --> H["validate_payer_wallet/1\n0x+40hex OR Solana base58 43-44"]
    G & H --> I{"any format errors?"}
    I -- "yes" --> J["{:error, {:invalid_format, [...]}}\n+ telemetry"]
    I -- "no (:ok)" --> K["validate_scheme/2"]
    K -- ":ok" --> L["{:ok, payload}\n+ telemetry"]
    K -- "error" --> M["{:error, {:invalid_upto_payment, reason}}\n+ telemetry"]
Loading

Reviews (7): Last reviewed commit: "fix: align Solana address regex {43,44} ..." | Re-trigger Greptile

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: New format validation breaks existing doctests and tests
    • Updated all test data and doctests to use valid 66-character EVM transaction hash format instead of placeholder '0xabc' to comply with new validation rules.

Create PR

Or push these changes by commenting:

@cursor push 3066308a51
Preview (3066308a51)
diff --git a/lib/x402.ex b/lib/x402.ex
--- a/lib/x402.ex
+++ b/lib/x402.ex
@@ -52,7 +52,7 @@
   ## Examples
 
       iex> payload = %{
-      ...>   "transactionHash" => "0xabc",
+      ...>   "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       ...>   "network" => "eip155:8453",
       ...>   "scheme" => "exact",
       ...>   "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -73,7 +73,7 @@
   ## Examples
 
       iex> X402.validate_payment_signature(%{
-      ...>   "transactionHash" => "0xabc",
+      ...>   "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       ...>   "network" => "eip155:8453",
       ...>   "scheme" => "exact",
       ...>   "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -82,7 +82,7 @@
         "network" => "eip155:8453",
         "payerWallet" => "0x1111111111111111111111111111111111111111",
         "scheme" => "exact",
-        "transactionHash" => "0xabc"
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
       }}
   """
   @spec validate_payment_signature(map()) ::
@@ -100,7 +100,7 @@
   ## Examples
 
       iex> payload = %{
-      ...>   "transactionHash" => "0xabc",
+      ...>   "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       ...>   "network" => "eip155:8453",
       ...>   "scheme" => "exact",
       ...>   "payerWallet" => "0x1111111111111111111111111111111111111111"

diff --git a/lib/x402/payment_signature.ex b/lib/x402/payment_signature.ex
--- a/lib/x402/payment_signature.ex
+++ b/lib/x402/payment_signature.ex
@@ -67,7 +67,7 @@
 
   ## Examples
 
-      iex> payload = %{"transactionHash" => "0xabc", "network" => "eip155:8453", "scheme" => "exact", "payerWallet" => "0x1111111111111111111111111111111111111111"}
+      iex> payload = %{"transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", "network" => "eip155:8453", "scheme" => "exact", "payerWallet" => "0x1111111111111111111111111111111111111111"}
       iex> value = payload |> Jason.encode!() |> Base.encode64()
       iex> X402.PaymentSignature.decode(value)
       {:ok, payload}
@@ -139,7 +139,7 @@
   ## Examples
 
       iex> payload = %{
-      ...>   "transactionHash" => "0xabc",
+      ...>   "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       ...>   "network" => "eip155:8453",
       ...>   "scheme" => "exact",
       ...>   "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -222,7 +222,7 @@
 
   ## Examples
 
-      iex> payload = %{"transactionHash" => "0xabc", "network" => "eip155:8453", "scheme" => "exact", "payerWallet" => "0x1111111111111111111111111111111111111111"}
+      iex> payload = %{"transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef", "network" => "eip155:8453", "scheme" => "exact", "payerWallet" => "0x1111111111111111111111111111111111111111"}
       iex> value = payload |> Jason.encode!() |> Base.encode64()
       iex> X402.PaymentSignature.decode_and_validate(value)
       {:ok, payload}

diff --git a/test/x402/payment_signature_test.exs b/test/x402/payment_signature_test.exs
--- a/test/x402/payment_signature_test.exs
+++ b/test/x402/payment_signature_test.exs
@@ -14,7 +14,7 @@
   describe "decode/1" do
     test "decodes a valid base64 json payload" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "exact",
         "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -58,7 +58,7 @@
   describe "validate/1" do
     test "returns ok for complete payload" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "exact",
         "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -92,7 +92,7 @@
 
     test "validates upto payments when value is within maxPrice from payload" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111",
@@ -107,7 +107,7 @@
   describe "validate/2" do
     test "validates upto payments when value is within requirements maxPrice" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111",
@@ -121,7 +121,7 @@
 
     test "rejects upto payments when value exceeds maxPrice" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111",
@@ -136,7 +136,7 @@
 
     test "rejects upto payments when value is missing" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -150,7 +150,7 @@
 
     test "returns invalid_payload for non-map requirements" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111",
@@ -164,7 +164,7 @@
   describe "decode_and_validate/1" do
     test "returns ok for valid encoded payload" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "exact",
         "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -191,7 +191,7 @@
   describe "decode_and_validate/2" do
     test "validates upto scheme against requirements" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "upto",
         "payerWallet" => "0x1111111111111111111111111111111111111111",

diff --git a/test/x402/plug/payment_gate_test.exs b/test/x402/plug/payment_gate_test.exs
--- a/test/x402/plug/payment_gate_test.exs
+++ b/test/x402/plug/payment_gate_test.exs
@@ -170,7 +170,7 @@
     assert result_conn.status == 200
 
     assert_receive {:verify_called, payload, requirements, hooks_module}
-    assert payload["transactionHash"] == "0xabc"
+    assert payload["transactionHash"] == "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef"
     assert requirements["network"] == "base-sepolia"
     assert requirements["asset"] == "USDC"
     assert requirements["resource"] == "/api/resource"
@@ -523,7 +523,7 @@
 
   defp valid_payment_header do
     %{
-      "transactionHash" => "0xabc",
+      "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       "network" => "base-sepolia",
       "scheme" => "exact",
       "payerWallet" => "0x1111111111111111111111111111111111111111"
@@ -534,7 +534,7 @@
 
   defp valid_upto_payment_header(value) do
     %{
-      "transactionHash" => "0xabc",
+      "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
       "network" => "base-sepolia",
       "scheme" => "upto",
       "payerWallet" => "0x1111111111111111111111111111111111111111",

diff --git a/test/x402/x402_test.exs b/test/x402/x402_test.exs
--- a/test/x402/x402_test.exs
+++ b/test/x402/x402_test.exs
@@ -12,7 +12,7 @@
 
     test "delegates payment-signature decode and validate" do
       payload = %{
-        "transactionHash" => "0xabc",
+        "transactionHash" => "0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef",
         "network" => "eip155:8453",
         "scheme" => "exact",
         "payerWallet" => "0x1111111111111111111111111111111111111111"

Comment thread lib/x402/payment_signature.ex Outdated
@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Ungrouped validate/2 clauses will fail CI compilation
    • Moved catch-all validate/2 clause to immediately after the guarded clause, grouping all validate/2 clauses together before private helper functions.

Create PR

Or push these changes by commenting:

@cursor push 17c0720b43
Preview (17c0720b43)
diff --git a/lib/x402/payment_signature.ex b/lib/x402/payment_signature.ex
--- a/lib/x402/payment_signature.ex
+++ b/lib/x402/payment_signature.ex
@@ -185,6 +185,11 @@
     end
   end
 
+  def validate(_payload, _requirements) do
+    Telemetry.emit(:payment_signature, :validate, :error, %{reason: :invalid_payload})
+    {:error, :invalid_payload}
+  end
+
   defp check_missing_fields(payload) do
     case missing_fields(payload) do
       [] ->
@@ -215,11 +220,6 @@
     end
   end
 
-  def validate(_payload, _requirements) do
-    Telemetry.emit(:payment_signature, :validate, :error, %{reason: :invalid_payload})
-    {:error, :invalid_payload}
-  end
-
   @doc since: "0.1.0", group: :verification
   @doc """
   Decodes and validates a `PAYMENT-SIGNATURE` header in one step.

Comment thread lib/x402/payment_signature.ex
@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Solana address regex too permissive vs Wallet module
    • Changed @solana_address_regex from {32,44} to {43,44} to match the Wallet module's security requirements and reject invalid Solana addresses.

Create PR

Or push these changes by commenting:

@cursor push 44ed5db8bc
Preview (44ed5db8bc)
diff --git a/lib/x402/payment_signature.ex b/lib/x402/payment_signature.ex
--- a/lib/x402/payment_signature.ex
+++ b/lib/x402/payment_signature.ex
@@ -27,8 +27,8 @@
   # Solana base-58 transaction signature (87–88 characters of base58 alphabet).
   @solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{87,88}$/
 
-  # Solana wallet address: base58, 32-44 characters.
-  @solana_address_regex ~r/^[1-9A-HJ-NP-Za-km-z]{32,44}$/
+  # Solana wallet address: base58, 43-44 characters.
+  @solana_address_regex ~r/^[1-9A-HJ-NP-Za-km-z]{43,44}$/
 
   # Single source of truth for the 8 KB decode guard — see X402.Header.
   @max_header_bytes X402.Header.max_header_bytes()

Comment thread lib/x402/payment_signature.ex Outdated
@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Solana tx signature regex rejects valid short signatures
    • Widened @solana_tx_sig_regex from {87,88} to {86,88} to accept valid 64-byte Ed25519 signatures with leading zero bytes that produce shorter base58 encodings.

Create PR

Or push these changes by commenting:

@cursor push 4c57e2ce96
Preview (4c57e2ce96)
diff --git a/lib/x402/payment_signature.ex b/lib/x402/payment_signature.ex
--- a/lib/x402/payment_signature.ex
+++ b/lib/x402/payment_signature.ex
@@ -21,11 +21,12 @@
   @eth_address_regex ~r/^0x[0-9a-fA-F]{40}$/
 
   # Transaction hash: 0x followed by exactly 64 hex characters (256-bit hash).
-  # Solana tx IDs are base58, 87-88 chars — we detect them by absence of "0x" prefix.
+  # Solana tx IDs are base58, 86-88 chars — we detect them by absence of "0x" prefix.
   @eth_tx_hash_regex ~r/^0x[0-9a-fA-F]{64}$/
 
-  # Solana base-58 transaction signature (87–88 characters of base58 alphabet).
-  @solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{87,88}$/
+  # Solana base-58 transaction signature (86–88 characters of base58 alphabet).
+  # 64-byte Ed25519 signatures with leading zero bytes produce shorter encodings.
+  @solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{86,88}$/
 
   # Solana wallet address: base58, 32-44 characters.
   @solana_address_regex ~r/^[1-9A-HJ-NP-Za-km-z]{32,44}$/
@@ -271,7 +272,7 @@
   end
 
   # Validates the format of fields that have known structural constraints:
-  #   - transactionHash: EVM 0x+64hex OR Solana base58 87-88 chars
+  #   - transactionHash: EVM 0x+64hex OR Solana base58 86-88 chars
   #   - payerWallet: EVM 0x+40hex OR Solana base58 32-44 chars
   #
   # We intentionally do NOT validate `network` and `scheme` here — those fields

@eth_tx_hash_regex ~r/^0x[0-9a-fA-F]{64}$/

# Solana base-58 transaction signature (87–88 characters of base58 alphabet).
@solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{87,88}$/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solana tx signature regex rejects valid short signatures

Low Severity

@solana_tx_sig_regex restricts Solana transaction signatures to exactly {87,88} base58 characters, but valid 64-byte Ed25519 signatures with leading zero bytes produce shorter base58 encodings (e.g., 86 characters with one leading zero byte). About ~0.4% of valid signatures could be incorrectly rejected. The proper approach (per Solana's own codebase) is to decode the base58 and verify the result is exactly 64 bytes, or widen the length range.

Fix in Cursor Fix in Web

@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Missing rejection_error clause for new invalid_format error
    • Added dedicated rejection_error clause for {:invalid_format, _} to return 'invalid payment payload' consistent with other validation errors.

Create PR

Or push these changes by commenting:

@cursor push 7279ba7876
Preview (7279ba7876)
diff --git a/lib/x402/plug/payment_gate.ex b/lib/x402/plug/payment_gate.ex
--- a/lib/x402/plug/payment_gate.ex
+++ b/lib/x402/plug/payment_gate.ex
@@ -531,6 +531,7 @@
     defp rejection_error(:already_exists), do: "payment already processed"
     defp rejection_error({:missing_fields, _fields}), do: "invalid payment payload"
     defp rejection_error({:invalid_upto_payment, _reason}), do: "invalid payment payload"
+    defp rejection_error({:invalid_format, _format_errors}), do: "invalid payment payload"
 
     defp rejection_error({:unexpected_facilitator_status, _status}),
       do: "facilitator rejected payment"

})

{:error, {:invalid_format, format_errors}}
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing rejection_error clause for new invalid_format error

Medium Severity

The new {:invalid_format, ...} error variant from check_field_formats has no dedicated rejection_error clause in payment_gate.ex. It falls through to the catch-all, returning "payment verification failed" instead of the more accurate "invalid payment payload" that other validation errors like {:missing_fields, _} and {:invalid_upto_payment, _} return. This gives clients a misleading error message for format failures.

Fix in Cursor Fix in Web

@cardotrejos

Copy link
Copy Markdown
Owner Author

@greptile review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 4 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Delegate specs missing new invalid_format error type
    • Added {:invalid_format, [{String.t(), atom()}]} to both delegate specs in lib/x402.ex to match the PaymentSignature.validate_error type.
  • ✅ Fixed: Wallet address regexes duplicated from Wallet module
    • Removed duplicated regexes and refactored validate_payer_wallet/1 to use X402.Wallet.valid?/1 directly.

Create PR

Or push these changes by commenting:

@cursor push 7f0a5aaed0
Preview (7f0a5aaed0)
diff --git a/lib/x402.ex b/lib/x402.ex
--- a/lib/x402.ex
+++ b/lib/x402.ex
@@ -90,7 +90,8 @@
           | {:error,
              :invalid_payload
              | {:missing_fields, [String.t()]}
-             | {:invalid_upto_payment, X402.PaymentSignature.upto_validation_error()}}
+             | {:invalid_upto_payment, X402.PaymentSignature.upto_validation_error()}
+             | {:invalid_format, [{String.t(), atom()}]}}
   defdelegate validate_payment_signature(payload), to: PaymentSignature, as: :validate
 
   @doc since: "0.1.0", group: :verification
@@ -115,7 +116,8 @@
              | :invalid_json
              | :invalid_payload
              | {:missing_fields, [String.t()]}
-             | {:invalid_upto_payment, X402.PaymentSignature.upto_validation_error()}}
+             | {:invalid_upto_payment, X402.PaymentSignature.upto_validation_error()}
+             | {:invalid_format, [{String.t(), atom()}]}}
   defdelegate decode_and_validate_payment_signature(header),
     to: PaymentSignature,
     as: :decode_and_validate

diff --git a/lib/x402/payment_signature.ex b/lib/x402/payment_signature.ex
--- a/lib/x402/payment_signature.ex
+++ b/lib/x402/payment_signature.ex
@@ -13,13 +13,10 @@
 
   alias X402.Telemetry
   alias X402.Utils
+  alias X402.Wallet
 
   @required_fields ~w(transactionHash network scheme payerWallet)
 
-  # EIP-55 Ethereum address: 0x followed by exactly 40 hex characters (case-insensitive).
-  # We accept mixed-case (checksummed) and lower-case (normalised) addresses.
-  @eth_address_regex ~r/^0x[0-9a-fA-F]{40}$/
-
   # Transaction hash: 0x followed by exactly 64 hex characters (256-bit hash).
   # Solana tx IDs are base58, 87-88 chars — we detect them by absence of "0x" prefix.
   @eth_tx_hash_regex ~r/^0x[0-9a-fA-F]{64}$/
@@ -27,9 +24,6 @@
   # Solana base-58 transaction signature (87–88 characters of base58 alphabet).
   @solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{87,88}$/
 
-  # Solana wallet address: base58, 32-44 characters.
-  @solana_address_regex ~r/^[1-9A-HJ-NP-Za-km-z]{43,44}$/
-
   # Single source of truth for the 8 KB decode guard — see X402.Header.
   @max_header_bytes X402.Header.max_header_bytes()
 
@@ -304,16 +298,10 @@
   @spec validate_payer_wallet(String.t() | nil) :: {String.t(), atom()} | nil
   defp validate_payer_wallet(nil), do: nil
 
-  defp validate_payer_wallet(wallet) when is_binary(wallet) do
-    cond do
-      Regex.match?(@eth_address_regex, wallet) -> nil
-      Regex.match?(@solana_address_regex, wallet) -> nil
-      true -> {"payerWallet", :invalid_format}
-    end
+  defp validate_payer_wallet(wallet) do
+    if Wallet.valid?(wallet), do: nil, else: {"payerWallet", :invalid_format}
   end
 
-  defp validate_payer_wallet(_), do: {"payerWallet", :invalid_format}
-
   @spec validate_scheme(map(), map()) ::
           :ok | {:error, {:invalid_upto_payment, upto_validation_error()}}
   defp validate_scheme(payload, requirements) do

Comment thread lib/x402.ex
"payerWallet" => "0x1111111111111111111111111111111111111111",
"scheme" => "exact",
"transactionHash" => "0xabc"
"transactionHash" => "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delegate specs missing new invalid_format error type

Medium Severity

The @spec for validate_payment_signature/1 and decode_and_validate_payment_signature/1 in lib/x402.ex don't include {:invalid_format, [{String.t(), atom()}]} as a possible error return. The underlying PaymentSignature.validate/1 now returns this new error type via check_field_formats/1, but the delegate specs still only list :invalid_payload, {:missing_fields, ...}, and {:invalid_upto_payment, ...}. Callers relying on these specs (or Dialyzer) won't know to handle the new error case.

Additional Locations (1)
Fix in Cursor Fix in Web

@solana_tx_sig_regex ~r/^[1-9A-HJ-NP-Za-km-z]{87,88}$/

# Solana wallet address: base58, 32-44 characters.
@solana_address_regex ~r/^[1-9A-HJ-NP-Za-km-z]{43,44}$/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wallet address regexes duplicated from Wallet module

Low Severity

@eth_address_regex and @solana_address_regex are identical copies of @evm_regex and @solana_regex from X402.Wallet, which already exposes valid_evm?/1 and valid_solana?/1 as public API. The validate_payer_wallet/1 function performs the exact same check as Wallet.valid?/1. If address format rules are updated in Wallet, the duplicated regexes here could silently diverge.

Fix in Cursor Fix in Web

@cardotrejos cardotrejos merged commit 33bebca into main Mar 28, 2026
4 checks passed
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.

1 participant