admin: reject non-canonical config array indices

admin.go

@@ -1142,6 +1142,22 @@ func handleStop(w http.ResponseWriter, r *http.Request) error {
 	return nil
 }
 
+func parseCanonicalArrayIndex(idx string) (int, error) {
+	if idx == "" {
+		return 0, fmt.Errorf("empty index")
+	}
+
+	i, err := strconv.Atoi(idx)
+	if err != nil {
+		return 0, err
+	}
+	if strconv.Itoa(i) != idx {
+		return 0, fmt.Errorf("non-canonical array index")
+	}
+
+	return i, nil
+}
+
 // unsyncedConfigAccess traverses into the current config and performs
 // the operation at path according to method, using body and out as
 // needed. This is a low-level, unsynchronized function; most callers
@@ -1203,11 +1219,12 @@ traverseLoop:
 				var idx int
 				if method != http.MethodPost {
 					idxStr := parts[len(parts)-1]
-					idx, err = strconv.Atoi(idxStr)
+					idx, err = parseCanonicalArrayIndex(idxStr)
 					if err != nil {
 						return fmt.Errorf("[%s] invalid array index '%s': %v",
 							path, idxStr, err)
 					}
+
 					if idx < 0 || (method != http.MethodPut && idx >= len(arr)) || idx > len(arr) {
 						return fmt.Errorf("[%s] array index out of bounds: %s", path, idxStr)
 					}
@@ -1307,7 +1324,7 @@ traverseLoop:
 			}
 
 		case []any:
-			partInt, err := strconv.Atoi(part)
+			partInt, err := parseCanonicalArrayIndex(part)
 			if err != nil {
 				return fmt.Errorf("[/%s] invalid array index '%s': %v",
 					strings.Join(parts[:i+1], "/"), part, err)

admin_test.go

@@ -19,6 +19,7 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"fmt"
+	"io"
 	"maps"
 	"net/http"
 	"net/http/httptest"
@@ -956,3 +957,37 @@ MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQDRS0LmTwUT0iwP
 		})
 	}
 }
+
+func TestUnsyncedConfigAccessCanonicalArrayIndices(t *testing.T) {
+	rawCfg = map[string]any{
+		rawConfigKey: map[string]any{
+			"list": []any{"zero", "one", "two", "three", "four", "five", "six", "seven", "eight", "nine", "ten"},
+		},
+	}
+
+	tests := []struct {
+		name    string
+		path    string
+		wantErr bool
+	}{
+		{name: "allow zero", path: "/" + rawConfigKey + "/list/0"},
+		{name: "allow one", path: "/" + rawConfigKey + "/list/1"},
+		{name: "allow ten", path: "/" + rawConfigKey + "/list/10"},
+		{name: "reject leading zero", path: "/" + rawConfigKey + "/list/01", wantErr: true},
+		{name: "reject multiple leading zeros", path: "/" + rawConfigKey + "/list/002", wantErr: true},
+		{name: "reject plus sign", path: "/" + rawConfigKey + "/list/+1", wantErr: true},
+		{name: "reject negative zero", path: "/" + rawConfigKey + "/list/-0", wantErr: true},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			err := unsyncedConfigAccess(http.MethodGet, tc.path, nil, io.Discard)
+			if tc.wantErr && err == nil {
+				t.Fatal("expected error, got nil")
+			}
+			if !tc.wantErr && err != nil {
+				t.Fatalf("expected no error, got %v", err)
+			}
+		})
+	}
+}