fix(volume): reject overflowing needle ID deltas (#10342)

* fix: reject overflowing needle ID deltas

Problem: Parsing a file ID with a delta can wrap a valid maximum needle ID back to zero without returning an error.

Root cause: Needle.ParsePath added the parsed uint64 delta without checking whether the sum exceeded the needle ID range.

Fix: Compare the delta with the remaining uint64 capacity before addition and return a contextual overflow error when it does not fit.

Validation: go test ./weed/storage/needle -run ^TestNeedleParsePathRejectsDeltaOverflow$ -count=1; go test ./weed/storage/needle -count=1; git diff --check 10cdaf381875492a2c752d1038797e96ff18208f..HEAD
Co-authored-by: Codex <noreply@openai.com>

* fix: propagate needle ID delta parse errors

Co-authored-by: Codex <noreply@openai.com>

* print the needle id in hex in the delta overflow error

* batch delete: keep processing after a cookie mismatch

* rust volume: reject overflowing needle id deltas

---------

Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>

seaweed-volume/src/storage/needle/needle.rs

@@ -712,7 +712,7 @@ fn format_needle_id_cookie(key: NeedleId, cookie: Cookie) -> String {
 /// Parse "needle_id_cookie_hex" or "needle_id_cookie_hex_delta" into (NeedleId, Cookie).
 /// Matches Go's ParsePath + ParseNeedleIdCookie: supports an optional `_delta` suffix
 /// where delta is a decimal number added to the NeedleId (used for sub-file addressing).
-/// Rejects strings that are too short or too long.
+/// Rejects strings that are too short or too long, and deltas that overflow the needle id.
 pub fn parse_needle_id_cookie(s: &str) -> Result<(NeedleId, Cookie), String> {
     // Go ParsePath: check for "_" suffix containing a decimal delta
     let (hex_part, delta) = if let Some(underscore_pos) = s.rfind('_') {
@@ -754,7 +754,11 @@ pub fn parse_needle_id_cookie(s: &str) -> Result<(NeedleId, Cookie), String> {
 
     // Apply delta if present (Go: n.Id += Uint64ToNeedleId(d))
     if let Some(d) = delta {
-        key = NeedleId(key.0.wrapping_add(d));
+        key = NeedleId(
+            key.0
+                .checked_add(d)
+                .ok_or_else(|| format!("delta {} overflows needle id {:x}", d, key.0))?,
+        );
     }
 
     Ok((key, cookie))
@@ -937,6 +941,13 @@ mod tests {
         assert_eq!(cookie, Cookie(0x12345678));
     }
 
+    #[test]
+    fn test_parse_rejects_delta_overflow() {
+        assert!(parse_needle_id_cookie("ffffffffffffffff00000000_1").is_err());
+        let (key, _) = parse_needle_id_cookie("fffffffffffffffe00000000_1").unwrap();
+        assert_eq!(key, NeedleId(u64::MAX));
+    }
+
     #[test]
     fn test_parse_odd_length_needle_id() {
         // Go's shell formats purge fids with strconv.FormatUint, which does not

weed/server/volume_grpc_batch_delete.go

@@ -35,7 +35,6 @@ func (vs *VolumeServer) BatchDelete(ctx context.Context, req *volume_server_pb.B
 
 		n := new(needle.Needle)
 		volumeId, _ := needle.NewVolumeId(vid)
-		ecVolume, isEcVolume := vs.store.FindEcVolume(volumeId)
 		if req.SkipCookieCheck {
 			n.Id, _, err = needle.ParseNeedleIdCookie(id_cookie)
 			if err != nil {
@@ -46,7 +45,17 @@ func (vs *VolumeServer) BatchDelete(ctx context.Context, req *volume_server_pb.B
 				continue
 			}
 		} else {
-			n.ParsePath(id_cookie)
+			if err := n.ParsePath(id_cookie); err != nil {
+				resp.Results = append(resp.Results, &volume_server_pb.DeleteResult{
+					FileId: fid,
+					Status: http.StatusBadRequest,
+					Error:  err.Error()})
+				continue
+			}
+		}
+
+		ecVolume, isEcVolume := vs.store.FindEcVolume(volumeId)
+		if !req.SkipCookieCheck {
 			cookie := n.Cookie
 			if !isEcVolume {
 				if _, err := vs.store.ReadVolumeNeedle(volumeId, n, nil, nil); err != nil {
@@ -73,7 +82,7 @@ func (vs *VolumeServer) BatchDelete(ctx context.Context, req *volume_server_pb.B
 					Status: http.StatusBadRequest,
 					Error:  "File Random Cookie does not match.",
 				})
-				break
+				continue
 			}
 		}
 

weed/server/volume_grpc_query.go

@@ -21,7 +21,10 @@ func (vs *VolumeServer) Query(req *volume_server_pb.QueryRequest, stream volume_
 
 		n := new(needle.Needle)
 		volumeId, _ := needle.NewVolumeId(vid)
-		n.ParsePath(id_cookie)
+		if err := n.ParsePath(id_cookie); err != nil {
+			glog.V(0).Infof("volume query failed to parse fid %s: %v", fid, err)
+			return err
+		}
 
 		cookie := n.Cookie
 		if _, err := vs.store.ReadVolumeNeedle(volumeId, n, nil, nil); err != nil {

weed/server/volume_server_handlers_write.go

@@ -83,7 +83,10 @@ func (vs *VolumeServer) DeleteHandler(w http.ResponseWriter, r *http.Request) {
 	n := new(needle.Needle)
 	vid, fid, _, _, _ := parseURLPath(r.URL.Path)
 	volumeId, _ := needle.NewVolumeId(vid)
-	n.ParsePath(fid)
+	if err := n.ParsePath(fid); err != nil {
+		writeJsonError(w, r, http.StatusBadRequest, err)
+		return
+	}
 
 	if !vs.maybeCheckJwtAuthorization(r, vid, fid, true) {
 		writeJsonError(w, r, http.StatusUnauthorized, errors.New("wrong jwt"))

weed/server/volume_server_parse_path_test.go

@@ -0,0 +1,45 @@
+package weed_server
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
+)
+
+const overflowingNeedleDeltaFid = "1,ffffffffffffffff00000000_1"
+
+func TestDeleteHandlerRejectsOverflowingNeedleDelta(t *testing.T) {
+	req := httptest.NewRequest(http.MethodDelete, "/"+overflowingNeedleDeltaFid, nil)
+	resp := httptest.NewRecorder()
+
+	(&VolumeServer{}).DeleteHandler(resp, req)
+
+	if resp.Code != http.StatusBadRequest {
+		t.Fatalf("DeleteHandler returned status %d, want %d", resp.Code, http.StatusBadRequest)
+	}
+}
+
+func TestBatchDeleteRejectsOverflowingNeedleDelta(t *testing.T) {
+	resp, err := (&VolumeServer{}).BatchDelete(context.Background(), &volume_server_pb.BatchDeleteRequest{
+		FileIds: []string{overflowingNeedleDeltaFid},
+	})
+	if err != nil {
+		t.Fatalf("BatchDelete returned error: %v", err)
+	}
+	if len(resp.Results) != 1 || resp.Results[0].Status != http.StatusBadRequest {
+		t.Fatalf("BatchDelete returned results %+v, want one bad request", resp.Results)
+	}
+}
+
+func TestQueryRejectsOverflowingNeedleDelta(t *testing.T) {
+	err := (&VolumeServer{}).Query(&volume_server_pb.QueryRequest{
+		FromFileIds: []string{overflowingNeedleDeltaFid},
+	}, nil)
+	if err == nil || !strings.Contains(err.Error(), "overflows needle id") {
+		t.Fatalf("Query returned error %v, want needle id overflow", err)
+	}
+}

weed/storage/needle/needle.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"math"
 	"net/http"
 	"strconv"
 	"strings"
@@ -135,6 +136,9 @@ func (n *Needle) ParsePath(fid string) (err error) {
 	}
 	if delta != "" {
 		if d, e := strconv.ParseUint(delta, 10, 64); e == nil {
+			if d > math.MaxUint64-uint64(n.Id) {
+				return fmt.Errorf("delta %d overflows needle id %x", d, n.Id)
+			}
 			n.Id += Uint64ToNeedleId(d)
 		} else {
 			return e

weed/storage/needle/needle_test.go

@@ -40,6 +40,14 @@ func TestParseKeyHash(t *testing.T) {
 	}
 }
 
+func TestNeedleParsePathRejectsDeltaOverflow(t *testing.T) {
+	var n Needle
+	err := n.ParsePath("ffffffffffffffff00000000_1")
+	if err == nil {
+		t.Fatalf("ParsePath accepted overflowing delta with needle id %d", n.Id)
+	}
+}
+
 func BenchmarkParseKeyHash(b *testing.B) {
 	b.ReportAllocs()