caddyauth: Don't write into shared hash buffer when building cache key (#8278)

HTTPBasicAuth.correctPassword built its hash cache key with
append(account.password, plaintextPassword...). account.password is
shared by all requests and can have spare capacity, so short passwords
were written into the stored hash's backing array, causing a data race
between concurrent requests and potentially mixed-up cache keys.

Build the key in a fresh buffer instead, and add regression tests.

modules/caddyhttp/caddyauth/basicauth.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	weakrand "math/rand/v2"
 	"net/http"
+	"slices"
 	"strings"
 	"sync"
 
@@ -173,7 +174,9 @@ func (hba HTTPBasicAuth) correctPassword(account Account, plaintextPassword []by
 	}
 
 	// compute a cache key that is unique for these input parameters
-	cacheKey := hex.EncodeToString(append(account.password, plaintextPassword...))
+	// (build it in a new buffer: appending to account.password could write
+	// into its spare capacity, which is shared by all concurrent requests)
+	cacheKey := hex.EncodeToString(slices.Concat(account.password, plaintextPassword))
 
 	// fast track: if the result of the input is already cached, use it
 	hba.HashCache.mu.RLock()

modules/caddyhttp/caddyauth/basicauth_test.go

@@ -19,6 +19,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"sort"
+	"sync"
 	"testing"
 
 	"github.com/caddyserver/caddy/v2"
@@ -140,3 +141,55 @@ func accountUsernames(hba HTTPBasicAuth) []string {
 	sort.Strings(names)
 	return names
 }
+
+func newCachedBasicAuth(t *testing.T) HTTPBasicAuth {
+	t.Helper()
+	hba := HTTPBasicAuth{
+		AccountList: []Account{{Username: "alice", Password: testBasicAuthHash}},
+		HashCache:   new(Cache),
+	}
+	ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
+	t.Cleanup(cancel)
+	if err := hba.Provision(ctx); err != nil {
+		t.Fatalf("provisioning failed: %v", err)
+	}
+	return hba
+}
+
+func TestBasicAuthHashCacheDoesNotModifyStoredHash(t *testing.T) {
+	hba := newCachedBasicAuth(t)
+	account := hba.Accounts["alice"]
+
+	// inspect the full backing array, including any spare capacity
+	before := append([]byte(nil), account.password[:cap(account.password)]...)
+
+	if _, err := hba.correctPassword(account, []byte("abcd")); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	after := account.password[:cap(account.password)]
+	if string(before) != string(after) {
+		t.Fatalf("stored password hash buffer was modified while building the cache key")
+	}
+}
+
+func TestBasicAuthHashCacheConcurrentRequests(t *testing.T) {
+	hba := newCachedBasicAuth(t)
+
+	passwords := []string{"correct horse", "aaa", "bbb", "ccc"}
+	var wg sync.WaitGroup
+	for i := range 64 {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			pw := passwords[i%len(passwords)]
+			req := httptest.NewRequest(http.MethodGet, "/", nil)
+			req.SetBasicAuth("alice", pw)
+			_, ok, _ := hba.Authenticate(httptest.NewRecorder(), req)
+			if want := pw == "correct horse"; ok != want {
+				t.Errorf("password %q: authenticated=%v, want %v", pw, ok, want)
+			}
+		}()
+	}
+	wg.Wait()
+}