From 28e46fc6d68a53a905664f3ad6b4db17c9621b19 Mon Sep 17 00:00:00 2001
From: Cyril Tovena <cyril.tovena@gmail.com>
Date: Thu, 25 Jul 2024 16:22:46 +0200
Subject: [PATCH 1/2] fix: Handle block offset exceeding chunk length in
 memchunk.go

The code change in memchunk.go fixes a bug where the block offset plus the length exceeds the chunk length. This bug is addressed by adding a check to ensure that the offset plus length does not exceed the chunk length before accessing the byte slice.

This seems to happen only when a chunk is corrupted and the cause is still unknown.
---
 pkg/chunkenc/memchunk.go                  | 3 +++
 pkg/storage/chunk/client/object_client.go | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/pkg/chunkenc/memchunk.go b/pkg/chunkenc/memchunk.go
index f6197888a854..3d87aec705dc 100644
--- a/pkg/chunkenc/memchunk.go
+++ b/pkg/chunkenc/memchunk.go
@@ -476,6 +476,9 @@ func newByteChunk(b []byte, blockSize, targetSize int, fromCheckpoint bool) (*Me
 			blk.uncompressedSize = db.uvarint()
 		}
 		l := db.uvarint()
+		if blk.offset+l > len(b) {
+			return nil, fmt.Errorf("block %d offset %d + length %d exceeds chunk length %d", i, blk.offset, l, len(b))
+		}
 		blk.b = b[blk.offset : blk.offset+l]
 
 		// Verify checksums.
diff --git a/pkg/storage/chunk/client/object_client.go b/pkg/storage/chunk/client/object_client.go
index 460c9566f6e7..f31bad01c5b9 100644
--- a/pkg/storage/chunk/client/object_client.go
+++ b/pkg/storage/chunk/client/object_client.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/base64"
+	"fmt"
 	"io"
 	"strings"
 	"time"
@@ -185,7 +186,7 @@ func (o *client) getChunk(ctx context.Context, decodeContext *chunk.DecodeContex
 	}
 
 	if err := c.Decode(decodeContext, buf.Bytes()); err != nil {
-		return chunk.Chunk{}, errors.WithStack(err)
+		return chunk.Chunk{}, errors.WithStack(fmt.Errorf("failed to decode chunk '%s': %w", key, err))
 	}
 	return c, nil
 }

From 1a19cce733212eedee40a2b99d6cf8f8e02103e8 Mon Sep 17 00:00:00 2001
From: Cyril Tovena <cyril.tovena@gmail.com>
Date: Fri, 26 Jul 2024 15:19:11 +0200
Subject: [PATCH 2/2] fix: Handle block offset exceeding chunk length in
 memchunk.go

---
 pkg/storage/chunk/fetcher/fetcher.go | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/pkg/storage/chunk/fetcher/fetcher.go b/pkg/storage/chunk/fetcher/fetcher.go
index cf763b9cbedc..45b6970045a9 100644
--- a/pkg/storage/chunk/fetcher/fetcher.go
+++ b/pkg/storage/chunk/fetcher/fetcher.go
@@ -9,7 +9,6 @@ import (
 	"github.com/opentracing/opentracing-go"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
-	"github.com/prometheus/prometheus/promql"
 
 	"github.com/grafana/loki/v3/pkg/logqlmodel/stats"
 	"github.com/grafana/loki/v3/pkg/storage/chunk"
@@ -218,8 +217,7 @@ func (c *Fetcher) FetchChunks(ctx context.Context, chunks []chunk.Chunk) ([]chun
 	}
 
 	if err != nil {
-		// Don't rely on Cortex error translation here.
-		return nil, promql.ErrStorage{Err: err}
+		level.Error(log).Log("msg", "failed downloading chunks", "err", err)
 	}
 
 	allChunks := append(fromCache, fromStorage...)