validate individual part ranges/size in upload/download

Author: lucix-aws

feature/s3/manager/download.go

@@ -77,6 +77,14 @@ type Downloader struct {
 	// operation requests made by the downloader.
 	ClientOptions []func(*s3.Options)
 
+	// By default, the downloader verifies that individual part ranges align
+	// based on the configured part size.
+	//
+	// You can disable that with this flag, however, Amazon S3 recommends
+	// against doing so because it damages the durability posture of object
+	// downloads.
+	DisableValidateParts bool
+
 	// Defines the buffer strategy used when downloading a part.
 	//
 	// If a WriterReadFromProvider is given the Download manager
@@ -404,6 +412,15 @@ func (d *downloader) tryDownloadChunk(params *s3.GetObjectInput, w io.Writer) (i
 	if err != nil {
 		return 0, err
 	}
+
+	if !d.cfg.DisableValidateParts && params.Range != nil && resp.ContentRange != nil {
+		expectStart, expectEnd := parseContentRange(*params.Range)
+		actualStart, actualEnd := parseContentRange(*resp.ContentRange)
+		if isRangeMismatch(expectStart, expectEnd, actualStart, actualEnd) {
+			return 0, fmt.Errorf("invalid content range: expect %d-%d, got %d-%d", expectStart, expectEnd, actualStart, actualEnd)
+		}
+	}
+
 	d.setTotalBytes(resp) // Set total if not yet set.
 	d.once.Do(func() {
 		d.etag = aws.ToString(resp.ETag)
@@ -422,6 +439,39 @@ func (d *downloader) tryDownloadChunk(params *s3.GetObjectInput, w io.Writer) (i
 	return n, nil
 }
 
+func parseContentRange(v string) (int, int) {
+	parts := strings.Split(v, "/") // chop the total off, if it's there
+
+	// we send "bytes=" but S3 appears to return "bytes ", handle both
+	trimmed := strings.TrimPrefix(parts[0], "bytes ")
+	trimmed = strings.TrimPrefix(trimmed, "bytes=")
+
+	parts = strings.Split(trimmed, "-")
+	if len(parts) != 2 {
+		return -1, -1
+	}
+
+	start, err := strconv.Atoi(parts[0])
+	if err != nil {
+		return -1, -1
+	}
+
+	end, err := strconv.Atoi(parts[1])
+	if err != nil {
+		return -1, -1
+	}
+
+	return start, end
+}
+
+func isRangeMismatch(expectStart, expectEnd, actualStart, actualEnd int) bool {
+	if expectStart == -1 || expectEnd == -1 || actualStart == -1 || actualEnd == -1 {
+		return false // we don't know, one of the ranges was missing or unparseable
+	}
+
+	return expectStart != actualStart && expectEnd != actualEnd
+}
+
 // getTotalBytes is a thread-safe getter for retrieving the total byte status.
 func (d *downloader) getTotalBytes() int64 {
 	d.m.Lock()

feature/s3/manager/upload.go

@@ -269,6 +269,14 @@ type Uploader struct {
 	// Note: S3 Express buckets always require CRC32 checksums regardless of this setting.
 	RequestChecksumCalculation aws.RequestChecksumCalculation
 
+	// By default, the uploader verifies that the number of expected uploaded
+	// parts matches the actual count at the end of an upload.
+	//
+	// You can disable that with this flag, however, Amazon S3 recommends
+	// against doing so because it damages the durability posture of object
+	// uploads.
+	DisableValidateParts bool
+
 	// partPool allows for the re-usage of streaming payload part buffers between upload calls
 	partPool byteSlicePool
 }
@@ -362,8 +370,9 @@ type uploader struct {
 
 	in *s3.PutObjectInput
 
-	readerPos int64 // current reader position
-	totalSize int64 // set to -1 if the size is not known
+	readerPos   int64 // current reader position
+	totalSize   int64 // set to -1 if the size is not known
+	expectParts int64
 }
 
 // internal logic for deciding whether to upload a single part or use a
@@ -446,6 +455,11 @@ func (u *uploader) initSize() error {
 			// during the size calculation. e.g odd number of bytes.
 			u.cfg.PartSize = (u.totalSize / int64(u.cfg.MaxUploadParts)) + 1
 		}
+
+		u.expectParts = u.totalSize / u.cfg.PartSize
+		if u.totalSize%u.cfg.PartSize != 0 {
+			u.expectParts++
+		}
 	}
 
 	return nil
@@ -877,6 +891,17 @@ func (u *multiuploader) complete() *s3.CompleteMultipartUploadOutput {
 		u.fail()
 	}
 
+	// expectParts == 0 means we didn't know the content length upfront and
+	// therefore we can't validate this at all
+	if u.expectParts == 0 || u.cfg.DisableValidateParts {
+		return resp
+	}
+
+	if len(u.parts) != int(u.expectParts) {
+		u.seterr(fmt.Errorf("uploaded part count mismatch: expected %d, got %d", u.expectParts, len(u.parts)))
+		u.fail()
+	}
+
 	return resp
 }