Security improvements (#234)

oskarszoon · web-flow · commit 83f156266ca9 · 2025-11-27T15:47:08.000+01:00
diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
@@ -92,6 +92,7 @@ jobs:
           repository: ${{ steps.pr.outputs.repo_full_name }}
           ref: ${{ steps.pr.outputs.head_sha }}
           fetch-depth: 0
+          persist-credentials: false  # Security: Don't store git credentials for fork code
 
       - name: Run Claude Code Review
         if: steps.pr.outputs.is_draft == 'false'
diff --git a/.github/workflows/cleanup_gh_cache.yaml b/.github/workflows/cleanup_gh_cache.yaml
@@ -1,5 +1,9 @@
 name: cleanup-gh-cache
 on: workflow_dispatch
+
+permissions:
+  actions: write  # Required for gh cache list and delete
+
 jobs:
   cleanup:
     runs-on: ubuntu-latest
diff --git a/.github/workflows/long_tests.yaml b/.github/workflows/long_tests.yaml
@@ -7,6 +7,9 @@ on:
         required: false
         type: string
 
+permissions:
+  contents: read  # Required for checkout
+
 env:
   REPO: teranode
   SETTINGS_CONTEXT_DEFAULT: test
diff --git a/go.mod b/go.mod
@@ -85,7 +85,7 @@ require (
 	github.com/bsv-blockchain/go-bc v1.0.2 // indirect
 	github.com/cenkalti/backoff/v5 v5.0.3 // indirect
 	github.com/containerd/containerd/api v1.9.0 // indirect
-	github.com/containerd/containerd/v2 v2.1.4 // indirect
+	github.com/containerd/containerd/v2 v2.1.5 // indirect
 	github.com/containerd/errdefs v1.0.0 // indirect
 	github.com/containerd/errdefs/pkg v0.3.0 // indirect
 	github.com/containerd/platforms v1.0.0-rc.2 // indirect
diff --git a/go.sum b/go.sum
@@ -213,8 +213,8 @@ github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/q
 github.com/containerd/console v1.0.5/go.mod h1:YynlIjWYF8myEu6sdkwKIvGQq+cOckRm6So2avqoYAk=
 github.com/containerd/containerd/api v1.9.0 h1:HZ/licowTRazus+wt9fM6r/9BQO7S0vD5lMcWspGIg0=
 github.com/containerd/containerd/api v1.9.0/go.mod h1:GhghKFmTR3hNtyznBoQ0EMWr9ju5AqHjcZPsSpTKutI=
-github.com/containerd/containerd/v2 v2.1.4 h1:/hXWjiSFd6ftrBOBGfAZ6T30LJcx1dBjdKEeI8xucKQ=
-github.com/containerd/containerd/v2 v2.1.4/go.mod h1:8C5QV9djwsYDNhxfTCFjWtTBZrqjditQ4/ghHSYjnHM=
+github.com/containerd/containerd/v2 v2.1.5 h1:pWSmPxUszaLZKQPvOx27iD4iH+aM6o0BoN9+hg77cro=
+github.com/containerd/containerd/v2 v2.1.5/go.mod h1:8C5QV9djwsYDNhxfTCFjWtTBZrqjditQ4/ghHSYjnHM=
 github.com/containerd/continuity v0.4.5 h1:ZRoN1sXq9u7V6QoHMcVWGhOwDFqZ4B9i5H6un1Wh0x4=
 github.com/containerd/continuity v0.4.5/go.mod h1:/lNJvtJKUQStBzpVQ1+rasXO1LAWtUQssk28EZvJ3nE=
 github.com/containerd/errdefs v1.0.0 h1:tg5yIfIlQIrxYtu9ajqY42W3lpS19XqdxRQeEwYG8PI=
diff --git a/services/asset/httpimpl/GetBlocks.go b/services/asset/httpimpl/GetBlocks.go
@@ -124,7 +124,13 @@ func (h *HTTP) GetBlocks(c echo.Context) error {
 	}
 
 	latestBlockHeight := blockMeta.Height
-	fromHeight := latestBlockHeight - uint32(offset) //nolint:gosec
+
+	// Validate offset doesn't exceed block height to prevent underflow
+	if uint32(offset) > latestBlockHeight {
+		return echo.NewHTTPError(http.StatusBadRequest, errors.NewInvalidArgumentError("offset exceeds block height").Error())
+	}
+
+	fromHeight := latestBlockHeight - uint32(offset)
 
 	h.logger.Debugf("[Asset_http] GetBlockChain for %s with offset = %d, limit = %d and fromHeight = %d", c.Request().RemoteAddr, offset, limit, fromHeight)
 
diff --git a/services/asset/httpimpl/helpers.go b/services/asset/httpimpl/helpers.go
@@ -80,6 +80,11 @@ func (h *HTTP) getLimitOffset(c echo.Context) (int, int, error) {
 		}
 	}
 
+	// Validate offset is non-negative to prevent underflow issues
+	if offset < 0 {
+		return 0, 0, echo.NewHTTPError(http.StatusBadRequest, errors.NewInvalidArgumentError("offset must be non-negative").Error())
+	}
+
 	limit := 20
 	limitStr := c.QueryParam("limit")
 
@@ -90,6 +95,11 @@ func (h *HTTP) getLimitOffset(c echo.Context) (int, int, error) {
 		}
 	}
 
+	// Validate limit bounds to prevent allocation issues
+	if limit < 1 {
+		limit = 1
+	}
+
 	if limit > 100 {
 		limit = 100
 	}
diff --git a/services/legacy/addrmgr/addrmanager.go b/services/legacy/addrmgr/addrmanager.go
@@ -774,7 +774,7 @@ func (a *AddrManager) AddAddressByIP(addrIP string) error {
 		return fmt.Errorf("invalid ip address %s", addr)
 	}
 
-	port, err := strconv.ParseUint(portStr, 10, 0)
+	port, err := strconv.ParseUint(portStr, 10, 16)
 	if err != nil {
 		return fmt.Errorf("invalid port %s: %v", portStr, err)
 	}
diff --git a/services/legacy/connmgr/seed.go b/services/legacy/connmgr/seed.go
@@ -58,8 +58,12 @@ func SeedFromDNS(chainParams *chaincfg.Params, reqServices wire.ServiceFlag,
 			}
 
 			addresses := make([]*wire.NetAddress, len(seedpeers))
-			// if this errors then we have *real* problems
-			intPort, _ := strconv.Atoi(chainParams.DefaultPort)
+			// Parse port as uint16 directly to avoid integer conversion issues
+			port, err := strconv.ParseUint(chainParams.DefaultPort, 10, 16)
+			if err != nil {
+				// Invalid port configuration, skip this seed
+				return
+			}
 
 			for i, peer := range seedpeers {
 				randSource := mrand.NewPCG(uint64(time.Now().UnixNano()), uint64(secondsIn4Days))
@@ -70,7 +74,7 @@ func SeedFromDNS(chainParams *chaincfg.Params, reqServices wire.ServiceFlag,
 					// a time randomly selected between 3
 					// and 7 days ago.
 					time.Now().Add(-1*time.Second*time.Duration(secondsIn3Days+rand.Int32N(secondsIn4Days))),
-					0, peer, uint16(intPort))
+					0, peer, uint16(port))
 			}
 
 			seedFn(addresses)
diff --git a/stores/blob/factory.go b/stores/blob/factory.go
@@ -167,13 +167,13 @@ func createDAHStore(storeURL *url.URL, logger ulogger.Logger, opts []options.Sto
 //   - error: Any error that occurred during creation, particularly if the batcher
 //     cannot be properly configured with the provided parameters
 func createBatchedStore(storeURL *url.URL, store Store, logger ulogger.Logger) (Store, error) {
-	sizeInBytes := int64(4 * 1024 * 1024)
+	sizeInBytes := 4 * 1024 * 1024
 
 	sizeString := storeURL.Query().Get("sizeInBytes")
 	if sizeString != "" {
 		var err error
 
-		sizeInBytes, err = strconv.ParseInt(sizeString, 10, 64)
+		sizeInBytes, err = strconv.Atoi(sizeString)
 		if err != nil {
 			return nil, errors.NewConfigurationError("error parsing batch size", err)
 		}
@@ -184,7 +184,7 @@ func createBatchedStore(storeURL *url.URL, store Store, logger ulogger.Logger) (
 		writeKeys = true
 	}
 
-	store = batcher.New(logger, store, int(sizeInBytes), writeKeys)
+	store = batcher.New(logger, store, sizeInBytes, writeKeys)
 
 	return store, nil
 }
diff --git a/stores/blob/file/file.go b/stores/blob/file/file.go
@@ -363,7 +363,7 @@ func newStore(logger ulogger.Logger, storeURL *url.URL, opts ...options.StoreOpt
 	options := options.NewStoreOptions(opts...)
 
 	if hashPrefix := storeURL.Query().Get("hashPrefix"); len(hashPrefix) > 0 {
-		val, err := strconv.ParseInt(hashPrefix, 10, 64)
+		val, err := strconv.ParseInt(hashPrefix, 10, 32)
 		if err != nil {
 			return nil, errors.NewStorageError("[File] failed to parse hashPrefix", err)
 		}
@@ -372,7 +372,7 @@ func newStore(logger ulogger.Logger, storeURL *url.URL, opts ...options.StoreOpt
 	}
 
 	if hashSuffix := storeURL.Query().Get("hashSuffix"); len(hashSuffix) > 0 {
-		val, err := strconv.ParseInt(hashSuffix, 10, 64)
+		val, err := strconv.ParseInt(hashSuffix, 10, 32)
 		if err != nil {
 			return nil, errors.NewStorageError("[File] failed to parse hashSuffix", err)
 		}
@@ -400,6 +400,11 @@ func newStore(logger ulogger.Logger, storeURL *url.URL, opts ...options.StoreOpt
 
 	// Check if longterm storage options are provided
 	if options.PersistSubDir != "" {
+		// Validate PersistSubDir doesn't contain path traversal sequences
+		if strings.Contains(options.PersistSubDir, "..") {
+			return nil, errors.NewInvalidArgumentError("[File] PersistSubDir contains path traversal sequence")
+		}
+
 		// Create persistent subdirectory
 		if err := os.MkdirAll(filepath.Join(path, options.PersistSubDir), 0755); err != nil {
 			return nil, errors.NewStorageError("[File] failed to create persist sub directory", err)
diff --git a/stores/blob/http/http.go b/stores/blob/http/http.go
@@ -336,12 +336,11 @@ func (s *HTTPStore) GetDAH(ctx context.Context, key []byte, fileType fileformat.
 	}
 
 	// Parse the DAH from the response body, the dah will be a string representation of an int
-	dahInt, err := strconv.Atoi(string(dah))
+	dahInt, err := strconv.ParseUint(string(dah), 10, 32)
 	if err != nil {
-		return 0, errors.NewStorageError("[HTTPStore] GetDAH failed to parse response body int", err)
+		return 0, errors.NewStorageError("[HTTPStore] GetDAH failed to parse response body", err)
 	}
 
-	// nolint: gosec
 	return uint32(dahInt), nil
 }
 
diff --git a/stores/blob/options/Options.go b/stores/blob/options/Options.go
@@ -10,6 +10,7 @@ import (
 	"os"
 	"path/filepath"
 	"strconv"
+	"strings"
 
 	"github.com/bsv-blockchain/teranode/errors"
 	"github.com/bsv-blockchain/teranode/pkg/fileformat"
@@ -277,12 +278,48 @@ func QueryToFileOptions(query url.Values) []FileOption {
 	return opts
 }
 
+// validatePathWithinBase ensures the resolved path stays within basePath to prevent
+// path traversal attacks. It resolves both paths to absolute form and checks that
+// the target path is a subdirectory of the base path.
+func validatePathWithinBase(basePath, targetPath string) error {
+	absBase, err := filepath.Abs(basePath)
+	if err != nil {
+		return err
+	}
+	absTarget, err := filepath.Abs(targetPath)
+	if err != nil {
+		return err
+	}
+
+	// Clean paths to remove any . or .. components
+	absBase = filepath.Clean(absBase)
+	absTarget = filepath.Clean(absTarget)
+
+	// Ensure target is within base (with proper separator handling)
+	// The target must either equal the base or start with base + separator
+	if absTarget != absBase && !strings.HasPrefix(absTarget, absBase+string(os.PathSeparator)) {
+		return errors.NewInvalidArgumentError("path escapes base directory")
+	}
+
+	return nil
+}
+
 func (o *Options) ConstructFilename(basePath string, key []byte, fileType fileformat.FileType) (string, error) {
 	var (
 		filename string
 		prefix   string
 	)
 
+	// Validate SubDirectory doesn't contain path traversal sequences
+	if strings.Contains(o.SubDirectory, "..") {
+		return "", errors.NewInvalidArgumentError("subdirectory contains path traversal sequence")
+	}
+
+	// Validate Filename doesn't contain path traversal or separator characters
+	if strings.Contains(o.Filename, "..") || strings.ContainsAny(o.Filename, `/\`) {
+		return "", errors.NewInvalidArgumentError("filename contains invalid path characters")
+	}
+
 	if len(o.Filename) > 0 {
 		filename = o.Filename
 	} else {
@@ -296,6 +333,11 @@ func (o *Options) ConstructFilename(basePath string, key []byte, fileType filefo
 	// Build the folder to use based on the StoreOption SubDirectory and the calculated prefix
 	folder := filepath.Join(basePath, o.SubDirectory, prefix)
 
+	// Validate the folder path stays within basePath
+	if err := validatePathWithinBase(basePath, folder); err != nil {
+		return "", err
+	}
+
 	// Create the folder if it doesn't exist but only if we have a prefix as the subdirectory
 	// would already have been created by StoreOptions
 	if prefix != "" {
@@ -304,9 +346,14 @@ func (o *Options) ConstructFilename(basePath string, key []byte, fileType filefo
 		}
 	}
 
-	filename = filepath.Join(folder, filename) + "." + fileType.String()
+	finalPath := filepath.Join(folder, filename) + "." + fileType.String()
+
+	// Final validation that the complete path stays within basePath
+	if err := validatePathWithinBase(basePath, finalPath); err != nil {
+		return "", err
+	}
 
-	return filename, nil
+	return finalPath, nil
 }
 
 func (o *Options) CalculatePrefix(filename string) string {
diff --git a/stores/blob/server.go b/stores/blob/server.go
@@ -474,13 +474,13 @@ func (s *HTTPBlobServer) handleSetDAH(w http.ResponseWriter, r *http.Request, op
 
 	dahStr := r.URL.Query().Get("dah")
 
-	dah, err := strconv.Atoi(dahStr)
+	dah, err := strconv.ParseUint(dahStr, 10, 32)
 	if err != nil {
 		http.Error(w, "Invalid DAH", http.StatusBadRequest)
 		return
 	}
 
-	err = s.store.SetDAH(r.Context(), key, fileType, uint32(dah), opts...) // nolint: gosec
+	err = s.store.SetDAH(r.Context(), key, fileType, uint32(dah), opts...)
 	if err != nil {
 		if err == errors.ErrNotFound {
 			http.Error(w, NotFoundMsg, http.StatusNotFound)
diff --git a/ui/dashboard/package-lock.json b/ui/dashboard/package-lock.json
diff --git a/ui/dashboard/package.json b/ui/dashboard/package.json

Original file line number	Diff line number	Diff line change
`@@ -80,6 +80,11 @@ func (h *HTTP) getLimitOffset(c echo.Context) (int, int, error) {`
`80`	`80`	`}`
`81`	`81`	`}`
`82`	`82`
	`83`	`+ // Validate offset is non-negative to prevent underflow issues`
	`84`	`+ if offset < 0 {`
	`85`	`+ return 0, 0, echo.NewHTTPError(http.StatusBadRequest, errors.NewInvalidArgumentError("offset must be non-negative").Error())`
	`86`	`+ }`
	`87`	`+`
`83`	`88`	`limit := 20`
`84`	`89`	`limitStr := c.QueryParam("limit")`
`85`	`90`
`@@ -90,6 +95,11 @@ func (h *HTTP) getLimitOffset(c echo.Context) (int, int, error) {`
`90`	`95`	`}`
`91`	`96`	`}`
`92`	`97`
	`98`	`+ // Validate limit bounds to prevent allocation issues`
	`99`	`+ if limit < 1 {`
	`100`	`+ limit = 1`
	`101`	`+ }`
	`102`	`+`
`93`	`103`	`if limit > 100 {`
`94`	`104`	`limit = 100`
`95`	`105`	`}`
Original file line number	Diff line number	Diff line change
`@@ -774,7 +774,7 @@ func (a *AddrManager) AddAddressByIP(addrIP string) error {`
`774`	`774`	`return fmt.Errorf("invalid ip address %s", addr)`
`775`	`775`	`}`
`776`	`776`
`777`		`- port, err := strconv.ParseUint(portStr, 10, 0)`
	`777`	`+ port, err := strconv.ParseUint(portStr, 10, 16)`
`778`	`778`	`if err != nil {`
`779`	`779`	`return fmt.Errorf("invalid port %s: %v", portStr, err)`
`780`	`780`	`}`