services/horizon: Validate transaction hash IDs as 64 lowecase hex chars

services/horizon/CHANGELOG.md

@@ -5,6 +5,8 @@ file. This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## Unreleased
 
+* Validate transaction hash IDs as 64 lowercase hex chars. As such, wrongly-formatted parameters which used to cause 404 (`Not found`) errors will now cause 400 (`Bad request`) HTTP errors.
+
 ## v1.0.1
 
 ### Fixed

services/horizon/internal/actions/helpers.go

@@ -2,12 +2,14 @@ package actions
 
 import (
 	"context"
+	"encoding/hex"
 	"fmt"
 	"mime"
 	"net/http"
 	"net/url"
 	"reflect"
 	"strconv"
+	"strings"
 	"unicode/utf8"
 
 	"github.com/asaskevich/govalidator"
@@ -94,14 +96,6 @@ func GetCursor(r *http.Request, name string) (string, error) {
 	return cursor, nil
 }
 
-// checkUTF8 checks if value is a valid UTF-8 string, otherwise sets
-// error to `action.Err`.
-func (base *Base) checkUTF8(name, value string) {
-	if err := checkUTF8(name, value); err != nil {
-		base.SetInvalidField(name, err)
-	}
-}
-
 func checkUTF8(name, value string) error {
 	if !utf8.ValidString(value) {
 		return problem.MakeInvalidFieldProblem(name, errors.New("invalid value"))
@@ -110,24 +104,32 @@ func checkUTF8(name, value string) error {
 }
 
 // GetStringFromURLParam retrieves a string from the URLParams.
-func (base *Base) GetStringFromURLParam(name string) string {
-	if base.Err != nil {
-		return ""
-	}
-
-	fromURL, ok := base.GetURLParam(name)
+func GetStringFromURLParam(r *http.Request, name string) (string, error) {
+	fromURL, ok := GetURLParam(r, name)
 	if ok {
 		ret, err := url.PathUnescape(fromURL)
 		if err != nil {
-			base.SetInvalidField(name, err)
-			return ""
+			return "", problem.MakeInvalidFieldProblem(name, err)
+		}
+
+		if err := checkUTF8(name, ret); err != nil {
+			return "", err
 		}
+		return ret, nil
+	}
+
+	return "", nil
+}
 
-		base.checkUTF8(name, ret)
-		return ret
+// GetStringFromURLParam retrieves a string from the URLParams.
+func (base *Base) GetStringFromURLParam(name string) string {
+	if base.Err != nil {
+		return ""
 	}
 
-	return ""
+	var ret string
+	ret, base.Err = GetString(base.R, name)
+	return ret
 }
 
 // GetString retrieves a string from either the URLParams, form or query string.
@@ -376,6 +378,23 @@ func (base *Base) GetAddress(name string, opts ...Opt) (result string) {
 	return result
 }
 
+// GetTransactionID retireves a transaction identifier by attempting to decode an hex-encoded,
+// 64-digit lowercase string at the provided name.
+func GetTransactionID(r *http.Request, name string) (string, error) {
+	value, err := GetStringFromURLParam(r, name)
+	if err != nil {
+		return "", err
+	}
+
+	if value != "" {
+		if _, err = hex.DecodeString(value); err != nil || len(value) != 64 || strings.ToLower(value) != value {
+			return "", problem.MakeInvalidFieldProblem(name, errors.New("invalid hash format"))
+		}
+	}
+
+	return value, nil
+}
+
 // GetAccountID retireves an xdr.AccountID by attempting to decode a stellar
 // address at the provided name.
 func GetAccountID(r *http.Request, name string) (xdr.AccountId, error) {

services/horizon/internal/actions/helpers_test.go

@@ -34,6 +34,25 @@ func TestGetAccountID(t *testing.T) {
 	)
 }
 
+func TestGetTransactionID(t *testing.T) {
+	tt := test.Start(t)
+	defer tt.Finish()
+	action := makeTestAction()
+
+	txID, err := GetTransactionID(action.R, "valid_tx_id")
+	tt.Assert.NoError(err)
+	tt.Assert.Equal(
+		"aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7cf",
+		txID,
+	)
+
+	txID, err = GetTransactionID(action.R, "invalid_uppercase_tx_id")
+	tt.Assert.Error(err)
+
+	txID, err = GetTransactionID(action.R, "invalid_too_short_tx_id")
+	tt.Assert.Error(err)
+}
+
 func TestGetAsset(t *testing.T) {
 	tt := test.Start(t)
 	defer tt.Finish()
@@ -711,28 +730,31 @@ func makeAction(path string, body map[string]string) *Base {
 
 func testURLParams() map[string]string {
 	return map[string]string{
-		"blank":                "",
-		"minus_one":            "-1",
-		"zero":                 "0",
-		"two":                  "2",
-		"twenty":               "20",
-		"32min":                fmt.Sprint(math.MinInt32),
-		"32max":                fmt.Sprint(math.MaxInt32),
-		"64min":                fmt.Sprint(math.MinInt64),
-		"64max":                fmt.Sprint(math.MaxInt64),
-		"native_asset_type":    "native",
-		"4_asset_type":         "credit_alphanum4",
-		"4_asset_code":         "USD",
-		"4_asset_issuer":       "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
-		"12_asset_type":        "credit_alphanum12",
-		"12_asset_code":        "USD",
-		"12_asset_issuer":      "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
-		"long_4_asset_type":    "credit_alphanum4",
-		"long_4_asset_code":    "SPOOON",
-		"long_4_asset_issuer":  "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
-		"long_12_asset_type":   "credit_alphanum12",
-		"long_12_asset_code":   "OHMYGODITSSOLONG",
-		"long_12_asset_issuer": "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
+		"blank":                   "",
+		"minus_one":               "-1",
+		"zero":                    "0",
+		"two":                     "2",
+		"twenty":                  "20",
+		"32min":                   fmt.Sprint(math.MinInt32),
+		"32max":                   fmt.Sprint(math.MaxInt32),
+		"64min":                   fmt.Sprint(math.MinInt64),
+		"64max":                   fmt.Sprint(math.MaxInt64),
+		"native_asset_type":       "native",
+		"4_asset_type":            "credit_alphanum4",
+		"4_asset_code":            "USD",
+		"4_asset_issuer":          "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
+		"12_asset_type":           "credit_alphanum12",
+		"12_asset_code":           "USD",
+		"12_asset_issuer":         "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
+		"long_4_asset_type":       "credit_alphanum4",
+		"long_4_asset_code":       "SPOOON",
+		"long_4_asset_issuer":     "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
+		"long_12_asset_type":      "credit_alphanum12",
+		"long_12_asset_code":      "OHMYGODITSSOLONG",
+		"long_12_asset_issuer":    "GBRPYHIL2CI3FNQ4BXLFMNDLFJUNPU2HY3ZMFSHONUCEOASW7QC7OX2H",
+		"valid_tx_id":             "aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7cf",
+		"invalid_uppercase_tx_id": "AA168F12124B7C196C0ADAEE7C73A64D37F99428CACB59A91FF389626845E7CF",
+		"invalid_too_short_tx_id": "aa168f12124b7c196c0adaee7c73a64d37f99428cacb59a91ff389626845e7",
 	}
 }
 

services/horizon/internal/actions_effects.go

@@ -109,7 +109,12 @@ func (action *EffectIndexAction) loadParams() {
 	action.PagingParams = action.GetPageQuery()
 	action.AccountFilter = action.GetAddress("account_id")
 	action.LedgerFilter = action.GetInt32("ledger_id")
-	action.TransactionFilter = action.GetString("tx_id")
+	var err error
+	action.TransactionFilter, err = actions.GetTransactionID(action.R, "tx_id")
+	if err != nil {
+		action.Err = err
+		return
+	}
 	action.OperationFilter = action.GetInt64("op_id")
 
 	filters, err := countNonEmpty(

services/horizon/internal/actions_effects_test.go

@@ -69,6 +69,20 @@ func TestEffectActions_Index(t *testing.T) {
 		}
 
 		// filtered by transaction
+		w = ht.Get("/transactions/2374e99349b9ef7dba9a5db3339b78fda8f34777b1af33ba468ad5c0df946d4d/effects")
+		if ht.Assert.Equal(200, w.Code) {
+			ht.Assert.PageOf(3, w.Body)
+		}
+		// missing tx
+		w = ht.Get("/transactions/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/effects")
+		ht.Assert.Equal(404, w.Code)
+		// uppercase tx hash not accepted
+		w = ht.Get("/transactions/2374E99349B9EF7DBA9A5DB3339B78FDA8F34777B1AF33BA468AD5C0DF946D4D/effects")
+		ht.Assert.Equal(400, w.Code)
+		// badly formated tx hash not accepted
+		w = ht.Get("/transactions/%00%1E4%5E%EF%BF%BD%EF%BF%BD%EF%BF%BDpVP%EF%BF%BDI&R%0BK%EF%BF%BD%1D%EF%BF%BD%EF%BF%BD=%EF%BF%BD%3F%23%EF%BF%BD%EF%BF%BDl%EF%BF%BD%1El%EF%BF%BD%EF%BF%BD/effects")
+		ht.Assert.Equal(400, w.Code)
+
 		w = ht.Get("/transactions/2374e99349b9ef7dba9a5db3339b78fda8f34777b1af33ba468ad5c0df946d4d/effects")
 		if ht.Assert.Equal(200, w.Code) {
 			ht.Assert.PageOf(3, w.Body)

services/horizon/internal/actions_operation.go

@@ -131,7 +131,12 @@ func (action *OperationIndexAction) loadParams() {
 	action.ValidateCursorAsDefault()
 	action.AccountFilter = action.GetAddress("account_id")
 	action.LedgerFilter = action.GetInt32("ledger_id")
-	action.TransactionFilter = action.GetStringFromURLParam("tx_id")
+	var err error
+	action.TransactionFilter, err = actions.GetTransactionID(action.R, "tx_id")
+	if err != nil {
+		action.Err = err
+		return
+	}
 	action.PagingParams = action.GetPageQuery()
 	action.IncludeFailed = action.GetBool("include_failed")
 	parsed, err := parseJoinField(&action.Action.Base)

services/horizon/internal/actions_operation_test.go

@@ -62,6 +62,15 @@ func TestOperationActions_Index(t *testing.T) {
 	if ht.Assert.Equal(200, w.Code) {
 		ht.Assert.PageOf(1, w.Body)
 	}
+	// missing tx
+	w = ht.Get("/transactions/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/operations")
+	ht.Assert.Equal(404, w.Code)
+	// uppercase tx hash not accepted
+	w = ht.Get("/transactions/2374E99349B9EF7DBA9A5DB3339B78FDA8F34777B1AF33BA468AD5C0DF946D4D/operations")
+	ht.Assert.Equal(400, w.Code)
+	// badly formated tx hash not accepted
+	w = ht.Get("/transactions/%00%1E4%5E%EF%BF%BD%EF%BF%BD%EF%BF%BDpVP%EF%BF%BDI&R%0BK%EF%BF%BD%1D%EF%BF%BD%EF%BF%BD=%EF%BF%BD%3F%23%EF%BF%BD%EF%BF%BDl%EF%BF%BD%1El%EF%BF%BD%EF%BF%BD/operations")
+	ht.Assert.Equal(400, w.Code)
 
 	w = ht.Get("/transactions/164a5064eba64f2cdbadb856bf3448485fc626247ada3ed39cddf0f6902133b6/operations")
 	if ht.Assert.Equal(200, w.Code) {

services/horizon/internal/actions_payment_test.go

@@ -56,6 +56,15 @@ func TestPaymentActions(t *testing.T) {
 	if ht.Assert.Equal(200, w.Code) {
 		ht.Assert.PageOf(0, w.Body)
 	}
+	// missing tx
+	w = ht.Get("/transactions/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff/payments")
+	ht.Assert.Equal(404, w.Code)
+	// uppercase tx hash not accepted
+	w = ht.Get("/transactions/2374E99349B9EF7DBA9A5DB3339B78FDA8F34777B1AF33BA468AD5C0DF946D4D/payments")
+	ht.Assert.Equal(400, w.Code)
+	// badly formated tx hash not accepted
+	w = ht.Get("/transactions/%00%1E4%5E%EF%BF%BD%EF%BF%BD%EF%BF%BDpVP%EF%BF%BDI&R%0BK%EF%BF%BD%1D%EF%BF%BD%EF%BF%BD=%EF%BF%BD%3F%23%EF%BF%BD%EF%BF%BDl%EF%BF%BD%1El%EF%BF%BD%EF%BF%BD/payments")
+	ht.Assert.Equal(400, w.Code)
 
 	// 400 for invalid tx hash
 	w = ht.Get("/transactions/ /payments")

services/horizon/internal/actions_transaction_test.go

@@ -30,8 +30,16 @@ func TestTransactionActions_Show(t *testing.T) {
 	}
 
 	// missing tx
-	w = ht.Get("/transactions/not_real")
+	w = ht.Get("/transactions/ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
 	ht.Assert.Equal(404, w.Code)
+
+	// uppercase tx hash not accepted
+	w = ht.Get("/transactions/2374E99349B9EF7DBA9A5DB3339B78FDA8F34777B1AF33BA468AD5C0DF946D4D")
+	ht.Assert.Equal(400, w.Code)
+
+	// badly formated tx hash not accepted
+	w = ht.Get("/transactions/%00%1E4%5E%EF%BF%BD%EF%BF%BD%EF%BF%BDpVP%EF%BF%BDI&R%0BK%EF%BF%BD%1D%EF%BF%BD%EF%BF%BD=%EF%BF%BD%3F%23%EF%BF%BD%EF%BF%BDl%EF%BF%BD%1El%EF%BF%BD%EF%BF%BD")
+	ht.Assert.Equal(400, w.Code)
 }
 
 func TestTransactionActions_Show_Failed(t *testing.T) {

services/horizon/internal/docs/reference/endpoints/effects-for-transaction.md

@@ -16,7 +16,7 @@ GET /transactions/{hash}/effects{?cursor,limit,order}
 
 | name | notes | description | example |
 | ---- | ----- | ----------- | ------- |
-| `hash` | required, string | A transaction hash, hex-encoded | `7e2050abc676003efc3eaadd623c927f753b7a6c37f50864bf284f4e1510d088` |
+| `hash` | required, string | A transaction hash, hex-encoded, lowercase. | `7e2050abc676003efc3eaadd623c927f753b7a6c37f50864bf284f4e1510d088` |
 | `?cursor` | optional, default _null_ | A paging token, specifying where to start returning records from. | `12884905984` |
 | `?order` | optional, string, default `asc` | The order in which to return rows, "asc" or "desc". | `asc` |
 | `?limit` | optional, number, default `10` | Maximum number of records to return. | `200` |

services/horizon/internal/docs/reference/endpoints/operations-for-transaction.md

@@ -22,7 +22,7 @@ GET /transactions/{hash}/operations{?cursor,limit,order}
 
 | name | notes | description | example |
 | ---- | ----- | ----------- | ------- |
-| `hash` | required, string | A transaction hash, hex-encoded | `4a3365180521e16b478d9f0c9198b97a9434fc9cb07b34f83ecc32fc54d0ca8a` |
+| `hash` | required, string | A transaction hash, hex-encoded, lowercase. | `4a3365180521e16b478d9f0c9198b97a9434fc9cb07b34f83ecc32fc54d0ca8a` |
 | `?cursor` | optional, default _null_ | A paging token, specifying where to start returning records from. | `12884905984` |
 | `?order` | optional, string, default `asc` | The order in which to return rows, "asc" or "desc". | `asc` |
 | `?limit` | optional, number, default `10` | Maximum number of records to return. | `200` |

services/horizon/internal/docs/reference/endpoints/payments-for-transaction.md

@@ -29,7 +29,7 @@ GET /transactions/{hash}/payments{?cursor,limit,order}
 
 | name | notes | description | example |
 | ---- | ----- | ----------- | ------- |
-| `hash` | required, string | A transaction hash, hex-encoded | `f65278b36875d170e865853838da400515f59ca23836f072e8d62cac18b803e5` |
+| `hash` | required, string | A transaction hash, hex-encoded, lowercase. | `f65278b36875d170e865853838da400515f59ca23836f072e8d62cac18b803e5` |
 | `?cursor` | optional, default _null_ | A paging token, specifying where to start returning records from. | `12884905984` |
 | `?order` | optional, string, default `asc` | The order in which to return rows, "asc" or "desc". | `asc` |
 | `?limit` | optional, number, default `10` | Maximum number of records to return. | `200` |

services/horizon/internal/docs/reference/endpoints/transactions-create.md

@@ -71,7 +71,7 @@ If the transaction failed or errored, then an error response will be returned. P
 
 | Name              | Type   |                                                                       |
 |-------------------|--------|-----------------------------------------------------------------------|
-| `hash`            | string | A hex-encoded hash of the submitted transaction.                      |
+| `hash`            | string | A hex-encoded, lowercase hash of the submitted transaction.                      |
 | `ledger`          | number | The ledger number that the submitted transaction was included in.     |
 | `envelope_xdr`    | string | A base64 encoded `TransactionEnvelope` [XDR](../xdr.md) object. |
 | `result_xdr`      | string | A base64 encoded `TransactionResult` [XDR](../xdr.md) object.   |

services/horizon/internal/docs/reference/endpoints/transactions-single.md

@@ -23,7 +23,7 @@ GET /transactions/{hash}
 
 |  name  |  notes  | description | example |
 | ------ | ------- | ----------- | ------- |
-| `hash` | required, string | A transaction hash, hex-encoded. | 264226cb06af3b86299031884175155e67a02e0a8ad0b3ab3a88b409a8c09d5c |
+| `hash` | required, string | A transaction hash, hex-encoded, lowercase. | 264226cb06af3b86299031884175155e67a02e0a8ad0b3ab3a88b409a8c09d5c |
 
 ### curl Example Request
 

services/horizon/internal/docs/reference/resources/ledger.md

@@ -12,7 +12,7 @@ To learn more about the concept of ledgers in the Stellar network, take a look a
 |------------------------------|--------|------------------------------------------------------------------------------------------------------------------------------|
 | id                           | string | The id is a unique identifier for this ledger.                                                                               |
 | paging_token                 | number | A [paging token](./page.md) suitable for use as a `cursor` parameter.                                                        |
-| hash                         | string | A hex-encoded SHA-256 hash of the ledger's [XDR](../../learn/xdr.md)-encoded form.                                           |
+| hash                         | string | A hex-encoded, lowercase SHA-256 hash of the ledger's [XDR](../../learn/xdr.md)-encoded form.                                           |
 | prev_hash                    | string | The hash of the ledger that chronologically came before this one.                                                            |
 | sequence                     | number | Sequence number of this ledger, suitable for use as the as the :id parameter for url templates that require a ledger number. |
 | successful_transaction_count | number | The number of successful transactions in this ledger.                                                                        |

services/horizon/internal/docs/reference/resources/transaction.md

@@ -15,7 +15,7 @@ To learn more about the concept of transactions in the Stellar network, take a l
 | id                      | string                   | The canonical id of this transaction, suitable for use as the :id parameter for url templates that require a transaction's ID. |
 | paging_token            | string                   | A [paging token](./page.md) suitable for use as the `cursor` parameter to transaction collection resources.                    |
 | successful              | bool                     | Indicates if transaction was successful or not.                                                                                |
-| hash                    | string                   | A hex-encoded SHA-256 hash of the transaction's [XDR](../../learn/xdr.md)-encoded form.                                        |
+| hash                    | string                   | A hex-encoded, lowercase SHA-256 hash of the transaction's [XDR](../../learn/xdr.md)-encoded form.                                        |
 | ledger                  | number                   | Sequence number of the ledger in which this transaction was applied.                                                           |
 | created_at              | ISO8601 string           |                                                                                                                                |
 | source_account          | string                   |                                                                                                                                |

services/horizon/internal/handler.go

@@ -203,7 +203,7 @@ func (we *web) streamIndexActionHandler(jfn interface{}, sfn streamFunc) http.Ha
 
 // showActionHandler handles all non-streamable endpoints.
 func showActionHandler(jfn interface{}) http.HandlerFunc {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	return func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()
 		contentType := render.Negotiate(r)
 		if jfn == nil || (contentType != render.MimeHal && contentType != render.MimeJSON) {
@@ -223,7 +223,7 @@ func showActionHandler(jfn interface{}) http.HandlerFunc {
 		}
 
 		h.ServeHTTP(w, r)
-	})
+	}
 }
 
 // getAccountID retrieves the account id by the provided key. The key is
@@ -240,8 +240,7 @@ func getAccountID(r *http.Request, key string, required bool) (string, error) {
 		return val, nil
 	}
 
-	_, err = strkey.Decode(strkey.VersionByteAccountID, val)
-	if err != nil {
+	if _, err = strkey.Decode(strkey.VersionByteAccountID, val); err != nil {
 		// TODO: add errInvalidValue
 		return "", problem.MakeInvalidFieldProblem(key, errors.New("invalid address"))
 	}
@@ -251,7 +250,7 @@ func getAccountID(r *http.Request, key string, required bool) (string, error) {
 
 // getShowActionQueryParams gets the available query params for all non-indexable endpoints.
 func getShowActionQueryParams(r *http.Request, requireAccountID bool) (*showActionQueryParams, error) {
-	txHash, err := hchi.GetStringFromURL(r, "tx_id")
+	txHash, err := actions.GetTransactionID(r, "tx_id")
 	if err != nil {
 		return nil, errors.Wrap(err, "getting tx id")
 	}