fix(security): validate account names to prevent Numscript injection (EN-1200)

pkg/api/handler_balances_create_test.go

@@ -28,7 +28,7 @@ var balanceCreateTestCases = []balanceCreateTestCase{
 	{
 		name: "nominal",
 		request: wallet.CreateBalance{
-			Name: uuid.NewString(),
+			Name: "balance1",
 		},
 	},
 	{
@@ -39,6 +39,32 @@ var balanceCreateTestCases = []balanceCreateTestCase{
 		expectedStatusCode: http.StatusBadRequest,
 		expectedErrorCode:  ErrorCodeValidation,
 	},
+	{
+		// The name contains valid characters but also an account separator;
+		// an unanchored regex would have accepted it, allowing address/script injection.
+		name: "with name containing an account separator",
+		request: wallet.CreateBalance{
+			Name: "balance:injected",
+		},
+		expectedStatusCode: http.StatusBadRequest,
+		expectedErrorCode:  ErrorCodeValidation,
+	},
+	{
+		name: "with name containing whitespace and numscript tokens",
+		request: wallet.CreateBalance{
+			Name: "x\n@world",
+		},
+		expectedStatusCode: http.StatusBadRequest,
+		expectedErrorCode:  ErrorCodeValidation,
+	},
+	{
+		// Dashes are allowed: dashed/UUID balance names must keep working.
+		// (They still alias under Address.String(); see chart.go.)
+		name: "with name containing a dash",
+		request: wallet.CreateBalance{
+			Name: "foo-bar",
+		},
+	},
 	{
 		name: "with reserved name",
 		request: wallet.CreateBalance{

pkg/api/handler_wallets_credit_test.go

@@ -5,6 +5,7 @@ import (
 	"math/big"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 
 	"github.com/formancehq/go-libs/v5/pkg/types/time"
@@ -102,6 +103,85 @@ func TestWalletsCredit(t *testing.T) {
 				}
 			},
 		},
+		{
+			name: "with wallet source containing numscript injection in balance",
+			request: wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+				Sources: []wallet.Subject{
+					wallet.NewWalletSubject("emitter1", "secondary\n@world"),
+				},
+			},
+			expectedStatusCode: http.StatusBadRequest,
+			expectedErrorCode:  ErrorCodeValidation,
+		},
+		{
+			name: "with wallet source spanning multiple account segments",
+			request: wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+				Sources: []wallet.Subject{
+					wallet.NewWalletSubject("emitter1", "balance:injected"),
+				},
+			},
+			expectedStatusCode: http.StatusBadRequest,
+			expectedErrorCode:  ErrorCodeValidation,
+		},
+		{
+			// Dashes are allowed in balance names (they still alias under
+			// Address.String(); see chart.go), so a dashed wallet source resolves.
+			name: "with dashed balance in wallet source",
+			request: wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+				Sources: []wallet.Subject{
+					wallet.NewWalletSubject("emitter1", "foo-bar"),
+				},
+			},
+			expectedPostTransaction: func(testEnv *testEnv, walletID string) wallet.PostTransaction {
+				return wallet.PostTransaction{
+					Script: &shared.V2PostTransactionScript{
+						Plain: pointer.For(wallet.BuildCreditWalletScript(
+							testEnv.Chart().GetBalanceAccount("emitter1", "foo-bar"),
+						)),
+						Vars: map[string]string{
+							"destination": testEnv.Chart().GetMainBalanceAccount(walletID),
+							"amount":      "USD 100",
+						},
+					},
+					Metadata: wallet.TransactionMetadata(nil),
+				}
+			},
+		},
+		{
+			name: "with wallet source containing numscript injection in identifier",
+			request: wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+				Sources: []wallet.Subject{
+					wallet.NewWalletSubject("emitter1 @world", ""),
+				},
+			},
+			expectedStatusCode: http.StatusBadRequest,
+			expectedErrorCode:  ErrorCodeValidation,
+		},
+		{
+			// Dashes are allowed in balance names (still alias under
+			// Address.String(); see chart.go), so a dashed destination resolves.
+			name: "with dashed destination balance",
+			request: wallet.CreditRequest{
+				Amount:  wallet.NewMonetary(big.NewInt(100), "USD"),
+				Balance: "foo-bar",
+			},
+			expectedPostTransaction: func(testEnv *testEnv, walletID string) wallet.PostTransaction {
+				return wallet.PostTransaction{
+					Script: &shared.V2PostTransactionScript{
+						Plain: pointer.For(wallet.BuildCreditWalletScript("world")),
+						Vars: map[string]string{
+							"destination": testEnv.Chart().GetBalanceAccount(walletID, "foo-bar"),
+							"amount":      "USD 100",
+						},
+					},
+					Metadata: wallet.TransactionMetadata(nil),
+				}
+			},
+		},
 		{
 			name: "with secondary balance as destination",
 			request: wallet.CreditRequest{
@@ -125,7 +205,7 @@ func TestWalletsCredit(t *testing.T) {
 			name: "with not existing secondary balance as destination",
 			request: wallet.CreditRequest{
 				Amount:  wallet.NewMonetary(big.NewInt(100), "USD"),
-				Balance: "not-existing",
+				Balance: "not_existing",
 			},
 			expectedStatusCode: http.StatusBadRequest,
 			expectedErrorCode:  ErrorCodeValidation,
@@ -158,6 +238,7 @@ func TestWalletsCredit(t *testing.T) {
 			t.Parallel()
 			walletID := uuid.NewString()
 			secondaryBalance := wallet.NewBalance("secondary", nil)
+			dashedBalance := wallet.NewBalance("foo-bar", nil)
 
 			req := newRequest(t, http.MethodPost, "/wallets/"+walletID+"/credit", testCase.request)
 			rec := httptest.NewRecorder()
@@ -173,13 +254,15 @@ func TestWalletsCredit(t *testing.T) {
 					return &testCase.postTransactionResult, nil
 				}),
 				WithGetAccount(func(ctx context.Context, ledger, account string) (*wallet.AccountWithVolumesAndBalances, error) {
-					if testEnv.Chart().GetBalanceAccount(walletID, secondaryBalance.Name) == account {
-						return &wallet.AccountWithVolumesAndBalances{
-							Account: wallet.Account{
-								Address:  account,
-								Metadata: metadataWithExpectingTypesAfterUnmarshalling(secondaryBalance.LedgerMetadata(walletID)),
-							},
-						}, nil
+					for _, b := range []wallet.Balance{secondaryBalance, dashedBalance} {
+						if testEnv.Chart().GetBalanceAccount(walletID, b.Name) == account {
+							return &wallet.AccountWithVolumesAndBalances{
+								Account: wallet.Account{
+									Address:  account,
+									Metadata: metadataWithExpectingTypesAfterUnmarshalling(b.LedgerMetadata(walletID)),
+								},
+							}, nil
+						}
 					}
 					return &wallet.AccountWithVolumesAndBalances{
 						Account: wallet.Account{
@@ -208,3 +291,42 @@ func TestWalletsCredit(t *testing.T) {
 		})
 	}
 }
+
+// TestWalletsCreditRejectsInvalidWalletID guards the WalletID supplied via the
+// URL path: a value spanning multiple account segments or carrying Numscript
+// tokens must be rejected before any ledger transaction is created.
+func TestWalletsCreditRejectsInvalidWalletID(t *testing.T) {
+	t.Parallel()
+
+	for _, walletID := range []string{
+		"wallet:injected",
+		"wallet\n@world",
+	} {
+		walletID := walletID
+		t.Run(walletID, func(t *testing.T) {
+			t.Parallel()
+
+			req := newRequest(t, http.MethodPost, "/wallets/"+url.PathEscape(walletID)+"/credit", wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+			})
+			rec := httptest.NewRecorder()
+
+			var (
+				testEnv            *testEnv
+				createdTransaction bool
+			)
+			testEnv = newTestEnv(
+				WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) {
+					createdTransaction = true
+					return &shared.V2Transaction{}, nil
+				}),
+			)
+			testEnv.Router().ServeHTTP(rec, req)
+
+			require.Equal(t, http.StatusBadRequest, rec.Result().StatusCode)
+			errorResponse := readErrorResponse(t, rec)
+			require.Equal(t, ErrorCodeValidation, errorResponse.ErrorCode)
+			require.False(t, createdTransaction)
+		})
+	}
+}

pkg/api/handler_wallets_debit_test.go

@@ -7,6 +7,7 @@ import (
 	"math/big"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"testing"
 
 	"github.com/formancehq/go-libs/v5/pkg/types/time"
@@ -182,6 +183,27 @@ var walletDebitTestCases = []testCase{
 			}
 		},
 	},
+	{
+		// Dashes are allowed in balance names (still alias under
+		// Address.String(); see chart.go), so a dashed balance source resolves.
+		name: "with dashed balance source",
+		request: wallet.DebitRequest{
+			Amount:   wallet.NewMonetary(big.NewInt(100), "USD"),
+			Balances: []string{"foo-bar"},
+		},
+		expectedPostTransaction: func(testEnv *testEnv, walletID string, h *wallet.DebitHold) wallet.PostTransaction {
+			return wallet.PostTransaction{
+				Script: &shared.V2PostTransactionScript{
+					Plain: pointer.For(wallet.BuildDebitWalletScript(map[string]map[string]string{}, testEnv.Chart().GetBalanceAccount(walletID, "foo-bar"))),
+					Vars: map[string]string{
+						"destination": "world",
+						"amount":      "USD 100",
+					},
+				},
+				Metadata: metadataWithExpectingTypesAfterUnmarshalling(wallet.TransactionMetadata(nil)),
+			}
+		},
+	},
 	{
 		name: "with wildcard balance as source",
 		request: wallet.DebitRequest{
@@ -318,6 +340,15 @@ func TestWalletsDebit(t *testing.T) {
 								}.LedgerMetadata(walletID)),
 							},
 						}, nil
+					case testEnv.Chart().GetBalanceAccount(walletID, "foo-bar"):
+						return &wallet.AccountWithVolumesAndBalances{
+							Account: wallet.Account{
+								Address: testEnv.Chart().GetBalanceAccount(walletID, "foo-bar"),
+								Metadata: metadataWithExpectingTypesAfterUnmarshalling(wallet.Balance{
+									Name: "foo-bar",
+								}.LedgerMetadata(walletID)),
+							},
+						}, nil
 					default:
 						return nil, errors.New("unexpected account: " + account)
 					}
@@ -416,3 +447,86 @@ func TestWalletsDebit(t *testing.T) {
 		})
 	}
 }
+
+func TestWalletsDebitRejectsInvalidBalanceMetadata(t *testing.T) {
+	t.Parallel()
+
+	walletID := uuid.NewString()
+	req := newRequest(t, http.MethodPost, "/wallets/"+walletID+"/debit", wallet.DebitRequest{
+		Amount:   wallet.NewMonetary(big.NewInt(100), "USD"),
+		Balances: []string{"legacy"},
+	})
+	rec := httptest.NewRecorder()
+
+	var (
+		createdTransaction bool
+		testEnv            *testEnv
+	)
+	testEnv = newTestEnv(
+		WithGetAccount(func(ctx context.Context, ledger, account string) (*wallet.AccountWithVolumesAndBalances, error) {
+			require.Equal(t, testEnv.Chart().GetBalanceAccount(walletID, "legacy"), account)
+			return &wallet.AccountWithVolumesAndBalances{
+				Account: wallet.Account{
+					Address: account,
+					// A stored balance name carrying Numscript tokens (e.g. tampered
+					// ledger metadata) must be rejected on read-back before any
+					// transaction is built. Dashes alone are valid, so use a real
+					// injection vector here.
+					Metadata: metadataWithExpectingTypesAfterUnmarshalling(wallet.Balance{
+						Name: "injected\n@world",
+					}.LedgerMetadata(walletID)),
+				},
+			}, nil
+		}),
+		WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) {
+			createdTransaction = true
+			return &shared.V2Transaction{}, nil
+		}),
+	)
+
+	testEnv.Router().ServeHTTP(rec, req)
+
+	require.Equal(t, http.StatusBadRequest, rec.Result().StatusCode)
+	errorResponse := readErrorResponse(t, rec)
+	require.Equal(t, ErrorCodeValidation, errorResponse.ErrorCode)
+	require.False(t, createdTransaction)
+}
+
+// TestWalletsDebitRejectsInvalidWalletID guards the WalletID supplied via the
+// URL path: a value spanning multiple account segments or carrying Numscript
+// tokens must be rejected before any ledger transaction is created.
+func TestWalletsDebitRejectsInvalidWalletID(t *testing.T) {
+	t.Parallel()
+
+	for _, walletID := range []string{
+		"wallet:injected",
+		"wallet\n@world",
+	} {
+		walletID := walletID
+		t.Run(walletID, func(t *testing.T) {
+			t.Parallel()
+
+			req := newRequest(t, http.MethodPost, "/wallets/"+url.PathEscape(walletID)+"/debit", wallet.DebitRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+			})
+			rec := httptest.NewRecorder()
+
+			var (
+				testEnv            *testEnv
+				createdTransaction bool
+			)
+			testEnv = newTestEnv(
+				WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) {
+					createdTransaction = true
+					return &shared.V2Transaction{}, nil
+				}),
+			)
+			testEnv.Router().ServeHTTP(rec, req)
+
+			require.Equal(t, http.StatusBadRequest, rec.Result().StatusCode)
+			errorResponse := readErrorResponse(t, rec)
+			require.Equal(t, ErrorCodeValidation, errorResponse.ErrorCode)
+			require.False(t, createdTransaction)
+		})
+	}
+}

pkg/balance.go

@@ -12,7 +12,16 @@ import (
 	"github.com/formancehq/go-libs/v5/pkg/types/time"
 )
 
-var balanceNameRegex = regexp.MustCompile("[0-9A-Za-z_-]+")
+// balanceNameRegex constrains user-supplied balance names to a single,
+// anchored ledger segment: it permits [0-9A-Za-z_-] but no ':' separator,
+// whitespace or Numscript metacharacters (the actual injection vectors).
+//
+// Dashes are allowed so legitimate dashed/UUID balance names keep working.
+// This does NOT make names collision-free: per the warning on
+// Address.String(), "foo-bar" still resolves to the same ledger account as
+// "foobar". Allowing dashes only prevents validation from rejecting existing
+// data — the aliasing itself is tracked as a separate ticket.
+var balanceNameRegex = regexp.MustCompile("^[0-9A-Za-z_-]+$")
 
 type CreateBalance struct {
 	WalletID  string     `json:"walletID"`