Mitigate DoS and fix dcrd orphan error handling

internal/webapi/middleware.go

@@ -184,76 +184,87 @@ func (w *WebAPI) vspMustBeOpen(c *gin.Context) {
 // Ticket hash, ticket hex, and parent hex are parsed from the request body and
 // validated. They are broadcast to the network using SendRawTransaction if dcrd
 // is not aware of them.
-func (w *WebAPI) broadcastTicket(c *gin.Context) {
-	const funcName = "broadcastTicket"
+func (w *WebAPI) broadcastTicket() gin.HandlerFunc {
+	// broadcastSem limits the number of concurrent transaction broadcasts.
+	broadcastSem := make(chan struct{}, 16)
 
-	// Read request bytes.
-	reqBytes, err := drainAndReplaceBody(c.Request)
-	if err != nil {
-		w.log.Warnf("%s: Error reading request (clientIP=%s): %v", funcName, c.ClientIP(), err)
-		w.sendErrorWithMsg(err.Error(), types.ErrBadRequest, c)
-		return
-	}
+	return func(c *gin.Context) {
+		const funcName = "broadcastTicket"
 
-	// Parse request to ensure ticket hash/hex and parent hex are included.
-	var request struct {
-		TicketHex  string `json:"tickethex" binding:"required"`
-		TicketHash string `json:"tickethash" binding:"required"`
-		ParentHex  string `json:"parenthex" binding:"required"`
-	}
-	if err := binding.JSON.BindBody(reqBytes, &request); err != nil {
-		w.log.Warnf("%s: Bad request (clientIP=%s): %v", funcName, c.ClientIP(), err)
-		w.sendErrorWithMsg(err.Error(), types.ErrBadRequest, c)
-		return
-	}
+		// Read request bytes.
+		reqBytes, err := drainAndReplaceBody(c.Request)
+		if err != nil {
+			w.log.Warnf("%s: Error reading request (clientIP=%s): %v", funcName, c.ClientIP(), err)
+			w.sendErrorWithMsg(err.Error(), types.ErrBadRequest, c)
+			return
+		}
 
-	// Ensure the provided ticket hex is a valid ticket.
-	msgTx, err := decodeTransaction(request.TicketHex)
-	if err != nil {
-		w.log.Errorf("%s: Failed to decode ticket hex (ticketHash=%s): %v",
-			funcName, request.TicketHash, err)
-		w.sendErrorWithMsg("cannot decode ticket hex", types.ErrBadRequest, c)
-		return
-	}
+		// Parse request to ensure ticket hash/hex and parent hex are included.
+		var request struct {
+			TicketHex  string `json:"tickethex" binding:"required"`
+			TicketHash string `json:"tickethash" binding:"required"`
+			ParentHex  string `json:"parenthex" binding:"required"`
+		}
+		if err := binding.JSON.BindBody(reqBytes, &request); err != nil {
+			w.log.Warnf("%s: Bad request (clientIP=%s): %v", funcName, c.ClientIP(), err)
+			w.sendErrorWithMsg(err.Error(), types.ErrBadRequest, c)
+			return
+		}
 
-	err = isValidTicket(msgTx)
-	if err != nil {
-		w.log.Warnf("%s: Invalid ticket (clientIP=%s, ticketHash=%s): %v",
-			funcName, c.ClientIP(), request.TicketHash, err)
-		w.sendError(types.ErrInvalidTicket, c)
-		return
-	}
+		// Ensure the provided ticket hex is a valid ticket.
+		msgTx, err := decodeTransaction(request.TicketHex)
+		if err != nil {
+			w.log.Errorf("%s: Failed to decode ticket hex (ticketHash=%s): %v",
+				funcName, request.TicketHash, err)
+			w.sendErrorWithMsg("cannot decode ticket hex", types.ErrBadRequest, c)
+			return
+		}
 
-	// Ensure hex matches hash.
-	if msgTx.TxHash().String() != request.TicketHash {
-		w.log.Warnf("%s: Ticket hex/hash mismatch (clientIP=%s, ticketHash=%s)",
-			funcName, c.ClientIP(), request.TicketHash)
-		w.sendErrorWithMsg("ticket hex does not match hash", types.ErrBadRequest, c)
-		return
-	}
+		err = isValidTicket(msgTx)
+		if err != nil {
+			w.log.Warnf("%s: Invalid ticket (clientIP=%s, ticketHash=%s): %v",
+				funcName, c.ClientIP(), request.TicketHash, err)
+			w.sendError(types.ErrInvalidTicket, c)
+			return
+		}
 
-	// Ensure the provided parent hex is a valid tx.
-	parentTx, err := decodeTransaction(request.ParentHex)
-	if err != nil {
-		w.log.Errorf("%s: Failed to decode parent hex (ticketHash=%s): %v", funcName, request.TicketHash, err)
-		w.sendErrorWithMsg("cannot decode parent hex", types.ErrBadRequest, c)
-		return
-	}
-	parentHash := parentTx.TxHash()
+		// Ensure hex matches hash.
+		if msgTx.TxHash().String() != request.TicketHash {
+			w.log.Warnf("%s: Ticket hex/hash mismatch (clientIP=%s, ticketHash=%s)",
+				funcName, c.ClientIP(), request.TicketHash)
+			w.sendErrorWithMsg("ticket hex does not match hash", types.ErrBadRequest, c)
+			return
+		}
 
-	// Check if local dcrd already knows the parent tx.
-	dcrdClient := c.MustGet(dcrdKey).(*rpc.DcrdRPC)
-	dcrdErr := c.MustGet(dcrdErrorKey)
-	if dcrdErr != nil {
-		w.log.Errorf("%s: %v", funcName, dcrdErr.(error))
-		w.sendError(types.ErrInternalError, c)
-		return
-	}
+		// Ensure the provided parent hex is a valid tx.
+		parentTx, err := decodeTransaction(request.ParentHex)
+		if err != nil {
+			w.log.Errorf("%s: Failed to decode parent hex (ticketHash=%s): %v", funcName, request.TicketHash, err)
+			w.sendErrorWithMsg("cannot decode parent hex", types.ErrBadRequest, c)
+			return
+		}
+		parentHash := parentTx.TxHash()
+
+		// Check if local dcrd already knows the parent tx.
+		dcrdClient := c.MustGet(dcrdKey).(*rpc.DcrdRPC)
+		dcrdErr := c.MustGet(dcrdErrorKey)
+		if dcrdErr != nil {
+			w.log.Errorf("%s: %v", funcName, dcrdErr.(error))
+			w.sendError(types.ErrInternalError, c)
+			return
+		}
+
+		_, err = dcrdClient.GetRawTransaction(parentHash.String())
+		if err != nil {
+			// Return error to the client if the error is not ErrNoTxInfo.
+			var e *wsrpc.Error
+			if !errors.As(err, &e) || e.Code != rpc.ErrNoTxInfo {
+				w.log.Errorf("%s: dcrd.GetRawTransaction for ticket parent failed (ticketHash=%s): %v",
+					funcName, request.TicketHash, err)
+				w.sendError(types.ErrInternalError, c)
+				return
+			}
 
-	_, err = dcrdClient.GetRawTransaction(parentHash.String())
-	if err != nil {
-		var e *wsrpc.Error
-		if errors.As(err, &e) && e.Code == rpc.ErrNoTxInfo {
 			// ErrNoTxInfo means local dcrd is not aware of the parent. We have
 			// the hex, so we can broadcast it here.
 
@@ -280,17 +291,30 @@ func (w *WebAPI) broadcastTicket(c *gin.Context) {
 				// Unknown output errors have special handling because they
 				// could be resolved by waiting for network propagation. Any
 				// other errors are returned to client immediately.
-				if !strings.Contains(err.Error(), rpc.ErrUnknownOutputs) {
+				if !rpc.ErrOrphan.MatchString(err.Error()) {
 					w.log.Errorf("%s: dcrd.SendRawTransaction for parent tx failed (ticketHash=%s): %v",
 						funcName, request.TicketHash, err)
 					w.sendError(types.ErrCannotBroadcastTicket, c)
 					return
 				}
 
+				// Only proceed with broadcast if semaphore slot is available.
+				// This reduces the impact of a malicious client sending
+				// requests which deliberately enter the txBroadcast loop.
+				select {
+				case broadcastSem <- struct{}{}:
+				default:
+					w.log.Warnf("%s: Too many pending broadcasts (ticketHash=%s)",
+						funcName, request.TicketHash)
+					w.sendError(types.ErrCannotBroadcastTicket, c)
+					return
+				}
+
 				w.log.Debugf("%s: Parent tx references an unknown output, waiting for it in mempool (ticketHash=%s)",
 					funcName, request.TicketHash)
 
 				txBroadcast := func() bool {
+					defer func() { <-broadcastSem }()
 					// Wait for 1 second and try again, max 7 attempts.
 					for range 7 {
 						time.Sleep(1 * time.Second)
@@ -310,38 +334,33 @@ func (w *WebAPI) broadcastTicket(c *gin.Context) {
 				}
 			}
 
-		} else {
-			w.log.Errorf("%s: dcrd.GetRawTransaction for ticket parent failed (ticketHash=%s): %v",
-				funcName, request.TicketHash, err)
-			w.sendError(types.ErrInternalError, c)
-			return
 		}
-	}
 
-	// Check if local dcrd already knows the ticket.
-	_, err = dcrdClient.GetRawTransaction(request.TicketHash)
-	if err == nil {
-		// No error means dcrd already knows the ticket, we are done here.
-		return
-	}
+		// Check if local dcrd already knows the ticket.
+		_, err = dcrdClient.GetRawTransaction(request.TicketHash)
+		if err == nil {
+			// No error means dcrd already knows the ticket, we are done here.
+			return
+		}
 
-	// ErrNoTxInfo means local dcrd is not aware of the ticket. We have the
-	// hex, so we can broadcast it here.
-	var e *wsrpc.Error
-	if errors.As(err, &e) && e.Code == rpc.ErrNoTxInfo {
-		w.log.Debugf("%s: Broadcasting ticket (ticketHash=%s)", funcName, request.TicketHash)
-		err = dcrdClient.SendRawTransaction(request.TicketHex)
-		if err != nil {
-			w.log.Errorf("%s: dcrd.SendRawTransaction for ticket failed (ticketHash=%s): %v",
+		// ErrNoTxInfo means local dcrd is not aware of the ticket. We have the
+		// hex, so we can broadcast it here.
+		var e *wsrpc.Error
+		if errors.As(err, &e) && e.Code == rpc.ErrNoTxInfo {
+			w.log.Debugf("%s: Broadcasting ticket (ticketHash=%s)", funcName, request.TicketHash)
+			err = dcrdClient.SendRawTransaction(request.TicketHex)
+			if err != nil {
+				w.log.Errorf("%s: dcrd.SendRawTransaction for ticket failed (ticketHash=%s): %v",
+					funcName, request.TicketHash, err)
+				w.sendError(types.ErrCannotBroadcastTicket, c)
+				return
+			}
+		} else {
+			w.log.Errorf("%s: dcrd.GetRawTransaction for ticket failed (ticketHash=%s): %v",
 				funcName, request.TicketHash, err)
-			w.sendError(types.ErrCannotBroadcastTicket, c)
+			w.sendError(types.ErrInternalError, c)
 			return
 		}
-	} else {
-		w.log.Errorf("%s: dcrd.GetRawTransaction for ticket failed (ticketHash=%s): %v",
-			funcName, request.TicketHash, err)
-		w.sendError(types.ErrInternalError, c)
-		return
 	}
 }
 

internal/webapi/payfee.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2021-2024 The Decred developers
+// Copyright (c) 2021-2026 The Decred developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -7,7 +7,6 @@ package webapi
 import (
 	"bytes"
 	"fmt"
-	"strings"
 	"time"
 
 	blockchain "github.com/decred/dcrd/blockchain/standalone/v2"
@@ -258,7 +257,7 @@ func (w *WebAPI) payFee(c *gin.Context) {
 			ticket.FeeTxStatus = database.FeeError
 
 			// Send the client an explicit error if the issue is unknown outputs.
-			if strings.Contains(err.Error(), rpc.ErrUnknownOutputs) {
+			if rpc.ErrOrphan.MatchString(err.Error()) {
 				w.sendError(types.ErrCannotBroadcastFeeUnknownOutputs, c)
 			} else {
 				w.sendError(types.ErrCannotBroadcastFee, c)

internal/webapi/webapi.go

@@ -246,11 +246,12 @@ func (w *WebAPI) router(cookieSecret []byte, dcrd rpc.DcrdConnect, wallets rpc.W
 	cookieStore := sessions.NewCookieStore(cookieSecret)
 
 	// API routes.
+	broadcastTicket := w.broadcastTicket()
 
 	api := router.Group("/api/v3")
 	api.GET("/vspinfo", w.requireWebCache, w.vspInfo)
-	api.POST("/setaltsignaddr", w.vspMustBeOpen, w.withDcrdClient(dcrd), w.broadcastTicket, w.vspAuth, w.setAltSignAddr)
-	api.POST("/feeaddress", w.vspMustBeOpen, w.withDcrdClient(dcrd), w.broadcastTicket, w.vspAuth, w.feeAddress)
+	api.POST("/setaltsignaddr", w.vspMustBeOpen, w.withDcrdClient(dcrd), broadcastTicket, w.vspAuth, w.setAltSignAddr)
+	api.POST("/feeaddress", w.vspMustBeOpen, w.withDcrdClient(dcrd), broadcastTicket, w.vspAuth, w.feeAddress)
 	api.POST("/ticketstatus", w.withDcrdClient(dcrd), w.vspAuth, w.ticketStatus)
 	api.POST("/payfee", w.vspMustBeOpen, w.withDcrdClient(dcrd), w.vspAuth, w.payFee)
 	api.POST("/setvotechoices", w.withDcrdClient(dcrd), w.withWalletClients(wallets), w.vspAuth, w.setVoteChoices)

rpc/dcrd.go

@@ -10,7 +10,7 @@ import (
 	"encoding/hex"
 	"errors"
 	"fmt"
-	"strings"
+	"regexp"
 
 	"github.com/decred/dcrd/blockchain/standalone/v2"
 	"github.com/decred/dcrd/chaincfg/chainhash"
@@ -29,11 +29,12 @@ const (
 	// we dont need to import the whole package.
 	ErrRPCDuplicateTx = -40
 	ErrNoTxInfo       = -5
-	// This error string is defined in dcrd/internal/mempool. Copied here
-	// because it is not exported.
-	ErrUnknownOutputs = "references outputs of unknown or fully-spent transaction"
 )
 
+// ErrOrphan error string is defined in dcrd/internal/mempool. Copied here
+// because it is not exported.
+var ErrOrphan = regexp.MustCompile(`orphan transaction \w+ references output \w+:\d+ of unknown or fully-spent transaction`)
+
 // DcrdRPC provides methods for calling dcrd JSON-RPCs without exposing the details
 // of JSON encoding.
 type DcrdRPC struct {
@@ -204,7 +205,7 @@ func (c *DcrdRPC) SendRawTransaction(txHex string) error {
 
 		// Errors about orphan/spent outputs indicate that dcrd *might* already
 		// have this transaction. Use getrawtransaction to confirm.
-		if strings.Contains(err.Error(), ErrUnknownOutputs) {
+		if ErrOrphan.MatchString(err.Error()) {
 			_, getErr := c.GetRawTransaction(txHex)
 			if getErr == nil {
 				return nil