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,
+	},
+	{
+		name: "with name containing a dash",
+		request: wallet.CreateBalance{
+			Name: "foo-bar",
+		},
+		expectedStatusCode: http.StatusBadRequest,
+		expectedErrorCode:  ErrorCodeValidation,
+	},
 	{
 		name: "with reserved name",
 		request: wallet.CreateBalance{

pkg/api/handler_wallets_credit_test.go

@@ -102,6 +102,59 @@ 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,
+		},
+		{
+			name: "with wallet source containing dash in balance",
+			request: wallet.CreditRequest{
+				Amount: wallet.NewMonetary(big.NewInt(100), "USD"),
+				Sources: []wallet.Subject{
+					wallet.NewWalletSubject("emitter1", "foo-bar"),
+				},
+			},
+			expectedStatusCode: http.StatusBadRequest,
+			expectedErrorCode:  ErrorCodeValidation,
+		},
+		{
+			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,
+		},
+		{
+			name: "with dash in destination balance",
+			request: wallet.CreditRequest{
+				Amount:  wallet.NewMonetary(big.NewInt(100), "USD"),
+				Balance: "foo-bar",
+			},
+			expectedStatusCode: http.StatusBadRequest,
+			expectedErrorCode:  ErrorCodeValidation,
+		},
 		{
 			name: "with secondary balance as destination",
 			request: wallet.CreditRequest{
@@ -125,7 +178,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,

pkg/api/handler_wallets_debit_test.go

@@ -182,6 +182,15 @@ var walletDebitTestCases = []testCase{
 			}
 		},
 	},
+	{
+		name: "with dash in balance source",
+		request: wallet.DebitRequest{
+			Amount:   wallet.NewMonetary(big.NewInt(100), "USD"),
+			Balances: []string{"foo-bar"},
+		},
+		expectedStatusCode: http.StatusBadRequest,
+		expectedErrorCode:  string(sdkerrors.SchemasErrorCodeValidation),
+	},
 	{
 		name: "with wildcard balance as source",
 		request: wallet.DebitRequest{
@@ -416,3 +425,43 @@ 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,
+					Metadata: metadataWithExpectingTypesAfterUnmarshalling(wallet.Balance{
+						Name: "foo-bar",
+					}.LedgerMetadata(walletID)),
+				},
+			}, nil
+		}),
+		WithCreateTransaction(func(ctx context.Context, ledger, ik string, p wallet.PostTransaction) (*shared.V2Transaction, error) {
+			createdTransaction = true
+			return nil, 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,11 @@ 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 (no ':' separator) and excludes '-' because
+// Address.String() strips dashes globally, making "foo-bar" and "foobar"
+// resolve to the same ledger account.
+var balanceNameRegex = regexp.MustCompile("^[0-9A-Za-z_]+$")
 
 type CreateBalance struct {
 	WalletID  string     `json:"walletID"`

pkg/credit.go

@@ -36,6 +36,9 @@ func (c CreditRequest) Validate() error {
 	if !assets.IsValid(c.Amount.Asset) {
 		return newErrInvalidAsset(c.Amount.Asset)
 	}
+	if c.Balance != "" && !balanceNameRegex.MatchString(c.Balance) {
+		return newErrInvalidAccountName(c.Balance)
+	}
 
 	return nil
 }

pkg/debit.go

@@ -55,6 +55,9 @@ func (d Debit) getDestination() Subject {
 }
 
 func (d Debit) Validate() error {
+	if !accountSegmentRegexp.MatchString(d.WalletID) {
+		return newErrInvalidAccountName(d.WalletID)
+	}
 	if d.Destination != nil {
 		if err := d.Destination.Validate(); err != nil {
 			return err
@@ -66,5 +69,10 @@ func (d Debit) Validate() error {
 	if !assets.IsValid(d.Amount.Asset) {
 		return newErrInvalidAsset(d.Amount.Asset)
 	}
+	for _, balance := range d.Balances {
+		if balance != "*" && !balanceNameRegex.MatchString(balance) {
+			return newErrInvalidAccountName(balance)
+		}
+	}
 	return nil
 }

pkg/manager.go

@@ -169,6 +169,9 @@ func (m *Manager) Debit(ctx context.Context, ik string, debit Debit) (*DebitHold
 		if balance.ExpiresAt != nil && !balance.ExpiresAt.IsZero() && balance.ExpiresAt.Before(time.Now()) {
 			continue
 		}
+		if !balanceNameRegex.MatchString(balance.Name) {
+			return nil, newErrInvalidAccountName(balance.Name)
+		}
 		sources = append(sources, m.chart.GetBalanceAccount(debit.WalletID, balance.Name))
 	}
 
@@ -284,6 +287,10 @@ func (m *Manager) Credit(ctx context.Context, ik string, credit Credit) error {
 		return err
 	}
 
+	if !accountSegmentRegexp.MatchString(credit.WalletID) {
+		return newErrInvalidAccountName(credit.WalletID)
+	}
+
 	if credit.Balance != "" {
 		if _, err := m.GetBalance(ctx, credit.WalletID, credit.Balance); err != nil {
 			return err

pkg/subject.go

@@ -2,6 +2,7 @@ package wallet
 
 import (
 	"fmt"
+	"regexp"
 
 	"github.com/formancehq/ledger/pkg/accounts"
 )
@@ -11,6 +12,13 @@ const (
 	SubjectTypeWallet        string = "WALLET"
 )
 
+// accountSegmentRegexp matches a single ledger account segment (no ':'
+// separator), anchored. WALLET identifiers and balance names are used as
+// individual chart segments, so — unlike a full ledger address — they must
+// not contain ':' (which would resolve to a nested account outside the
+// expected balance model and is the Numscript-injection vector).
+var accountSegmentRegexp = regexp.MustCompile("^" + accounts.SegmentRegex + "$")
+
 type Subject struct {
 	Type       string `json:"type"`
 	Identifier string `json:"identifier"`
@@ -43,6 +51,14 @@ func (s Subject) Validate() error {
 		if s.Identifier == "" {
 			return fmt.Errorf("wallet identifier cannot be empty")
 		}
+		if !accountSegmentRegexp.MatchString(s.Identifier) {
+			return newErrInvalidAccountName(s.Identifier)
+		}
+		// Balance names are stricter than wallet identifiers because
+		// Address.String() strips dashes globally, creating aliases.
+		if s.Balance != "" && !balanceNameRegex.MatchString(s.Balance) {
+			return newErrInvalidAccountName(s.Balance)
+		}
 	}
 	return nil
 }