File 0006-Fix-avatar.patch of Package grafana
From 478097400daaa01ecce07dcd3d43129628409542 Mon Sep 17 00:00:00 2001
From: Matheus Macabu <macabu.matheus@gmail.com>
Date: Tue, 27 Jan 2026 09:23:21 +0100
Subject: [PATCH] Avatar: Require sign-in, remove queue, respect timeout
Fixes CVE-2026-21720
---
pkg/api/api.go | 2 +-
pkg/api/avatar/avatar.go | 83 ++++++++++++++++++++++-------------
pkg/api/avatar/avatar_test.go | 13 +++---
3 files changed, 61 insertions(+), 37 deletions(-)
diff --git a/pkg/api/api.go b/pkg/api/api.go
index 61ced28f2764e..8c504ddf60ad4 100644
--- a/pkg/api/api.go
+++ b/pkg/api/api.go
@@ -606,7 +606,7 @@ func (hs *HTTPServer) registerRoutes() {
r.Any("/api/gnet/*", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), reqSignedIn, hs.ProxyGnetRequest)
// Gravatar service
- r.Get("/avatar/:hash", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), hs.AvatarCacheServer.Handler)
+ r.Get("/avatar/:hash", requestmeta.SetSLOGroup(requestmeta.SLOGroupHighSlow), reqSignedIn, hs.AvatarCacheServer.Handler)
// Snapshots
r.Get("/api/snapshot/shared-options/", reqSignedIn, hs.GetSharingOptions)
diff --git a/pkg/api/avatar/avatar.go b/pkg/api/avatar/avatar.go
index 864d6187af072..38acf648b549e 100644
--- a/pkg/api/avatar/avatar.go
+++ b/pkg/api/avatar/avatar.go
@@ -7,8 +7,7 @@
package avatar
import (
- "bufio"
- "bytes"
+ "context"
"fmt"
"io"
"net/http"
@@ -20,7 +19,7 @@ import (
"sync"
"time"
- gocache "github.com/patrickmn/go-cache"
+ lru "github.com/hashicorp/golang-lru/v2/expirable"
"github.com/grafana/grafana/pkg/infra/log"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
@@ -35,7 +34,7 @@ const (
// Avatar represents the avatar object.
type Avatar struct {
hash string
- data *bytes.Buffer
+ data []byte
notFound bool
isCustom bool
timestamp time.Time
@@ -67,13 +66,16 @@ func (a *Avatar) Expired() bool {
}
func (a *Avatar) Encode(wr io.Writer) error {
- _, err := wr.Write(a.data.Bytes())
+ _, err := wr.Write(a.data)
return err
}
-func (a *Avatar) update(baseUrl string) (err error) {
+func (a *Avatar) update(ctx context.Context, baseUrl string) (err error) {
customUrl := baseUrl + a.hash + "?"
- select {
+ if true {
+ return avatarFetch(ctx, a, customUrl)
+ }
+ select { //nolint:govet
case <-time.After(time.Second * 3):
err = fmt.Errorf("get gravatar image %s timeout", a.hash)
case err = <-thunder.GoFetch(customUrl, a):
@@ -94,7 +96,7 @@ func (a *Avatar) setAvatarNotFound() {
type AvatarCacheServer struct {
cfg *setting.Cfg
notFound *Avatar
- cache *gocache.Cache
+ cache *lru.LRU[string, *Avatar]
}
var validMD5 = regexp.MustCompile("^[a-fA-F0-9]{32}$")
@@ -107,12 +109,12 @@ func (a *AvatarCacheServer) Handler(ctx *contextmodel.ReqContext) {
return
}
- avatar := a.GetAvatarForHash(a.cfg, hash)
+ avatar := a.GetAvatarForHashContext(ctx.Req.Context(), a.cfg, hash)
ctx.Resp.Header().Set("Content-Type", "image/jpeg")
if !a.cfg.EnableGzip {
- ctx.Resp.Header().Set("Content-Length", strconv.Itoa(len(avatar.data.Bytes())))
+ ctx.Resp.Header().Set("Content-Length", strconv.Itoa(len(avatar.data)))
}
ctx.Resp.Header().Set("Cache-Control", "private, max-age=3600")
@@ -124,25 +126,33 @@ func (a *AvatarCacheServer) Handler(ctx *contextmodel.ReqContext) {
}
func (a *AvatarCacheServer) GetAvatarForHash(cfg *setting.Cfg, hash string) *Avatar {
+ return a.GetAvatarForHashContext(context.TODO(), cfg, hash)
+}
+
+func (a *AvatarCacheServer) GetAvatarForHashContext(ctx context.Context, cfg *setting.Cfg, hash string) *Avatar {
if cfg.DisableGravatar {
alog.Warn("'GetGravatarForHash' called despite gravatars being disabled; returning default profile image")
return a.notFound
}
- return a.getAvatarForHash(hash, gravatarSource)
+ return a.getAvatarForHashContext(ctx, hash, gravatarSource)
}
func (a *AvatarCacheServer) getAvatarForHash(hash string, baseUrl string) *Avatar {
+ return a.getAvatarForHashContext(context.TODO(), hash, baseUrl)
+}
+
+func (a *AvatarCacheServer) getAvatarForHashContext(ctx context.Context, hash string, baseUrl string) *Avatar {
var avatar *Avatar
obj, exists := a.cache.Get(hash)
if exists {
- avatar = obj.(*Avatar)
+ avatar = obj
} else {
avatar = New(hash)
}
- if avatar.Expired() {
+ if !exists {
// The cache item is either expired or newly created, update it from the server
- if err := avatar.update(baseUrl); err != nil {
+ if err := avatar.update(ctx, baseUrl); err != nil {
alog.Debug("avatar update", "err", err)
avatar = a.notFound
}
@@ -151,8 +161,8 @@ func (a *AvatarCacheServer) getAvatarForHash(hash string, baseUrl string) *Avata
if avatar.notFound {
avatar = a.notFound
} else if !exists {
- if err := a.cache.Add(hash, avatar, gocache.DefaultExpiration); err != nil {
- alog.Debug("add avatar to cache", "err", err)
+ if evicted := a.cache.Add(hash, avatar); evicted {
+ alog.Debug("add avatar to cache", "evicted", evicted)
}
}
return avatar
@@ -171,7 +181,7 @@ func newCacheServer(cfg *setting.Cfg) *AvatarCacheServer {
return &AvatarCacheServer{
cfg: cfg,
notFound: newNotFound(cfg),
- cache: gocache.New(time.Hour, time.Hour*2),
+ cache: lru.NewLRU[string, *Avatar](2000, nil, time.Hour),
}
}
@@ -192,7 +202,7 @@ func newNotFound(cfg *setting.Cfg) *Avatar {
if data, err := os.ReadFile(path); err != nil {
alog.Error("Failed to read user_profile.png", "path", path)
} else {
- avatar.data = bytes.NewBuffer(data)
+ avatar.data = data
}
return avatar
@@ -264,15 +274,18 @@ var client = &http.Client{
// Break out the fetch function in a way that makes each
// Portion highly reusable
func (a *thunderTask) fetch() error {
- a.Avatar.timestamp = time.Now()
+ return avatarFetch(context.TODO(), a.Avatar, a.BaseUrl)
+}
- alog.Debug("avatar.fetch(fetch new avatar)", "url", a.BaseUrl)
- // First do the fetch to get the Gravatar with a retro icon fallback
- err := performGet(a.BaseUrl+gravatarReqParams, a.Avatar, getGravatarHandler)
+func avatarFetch(ctx context.Context, avatar *Avatar, baseURL string) error {
+ avatar.timestamp = time.Now()
+ alog.Debug("avatar.fetch(fetch new avatar)", "url", baseURL)
+ // First do the fetch to get the Gravatar with a retro icon fallback
+ err := performGet(ctx, baseURL+gravatarReqParams, avatar, getGravatarHandler)
if err == nil {
// Next do a fetch with a 404 fallback to see if it's a custom gravatar
- return performGet(a.BaseUrl+hasCustomReqParams, a.Avatar, checkIsCustomHandler)
+ return performGet(ctx, baseURL+hasCustomReqParams, avatar, checkIsCustomHandler)
}
return err
}
@@ -286,11 +299,21 @@ func getGravatarHandler(av *Avatar, resp *http.Response) error {
return fmt.Errorf("status code: %d", resp.StatusCode)
}
- av.data = &bytes.Buffer{}
- writer := bufio.NewWriter(av.data)
-
- _, err := io.Copy(writer, resp.Body)
- return err
+ var (
+ data []byte
+ err error
+ )
+ if resp.ContentLength > 0 {
+ data = make([]byte, resp.ContentLength)
+ _, err = io.ReadFull(resp.Body, data)
+ } else {
+ data, err = io.ReadAll(resp.Body)
+ }
+ if err != nil {
+ return err
+ }
+ av.data = data
+ return nil
}
// Uses the d=404 fallback to see if the gravatar we got back is custom
@@ -300,8 +323,8 @@ func checkIsCustomHandler(av *Avatar, resp *http.Response) error {
}
// Reusable Get helper that allows us to pass in custom handling depending on the endpoint
-func performGet(url string, av *Avatar, handler ResponseHandler) error {
- req, err := http.NewRequest("GET", url, nil)
+func performGet(ctx context.Context, url string, av *Avatar, handler ResponseHandler) error {
+ req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
if err != nil {
return err
}
diff --git a/pkg/api/avatar/avatar_test.go b/pkg/api/avatar/avatar_test.go
index 916ba082e2211..625473d190a38 100644
--- a/pkg/api/avatar/avatar_test.go
+++ b/pkg/api/avatar/avatar_test.go
@@ -23,14 +23,14 @@ func TestAvatar_AvatarRetrieval(t *testing.T) {
mockServer := setupMockGravatarServer(&callCounter, false)
t.Cleanup(func() {
- avc.cache.Flush()
+ avc.cache.Purge()
mockServer.Close()
})
av := avc.getAvatarForHash(DEFAULT_NONSENSE_HASH, mockServer.URL+"/avatar/")
// verify there was a call to get the image and a call to the 404 fallback
require.Equal(t, callCounter, 2)
- require.Equal(t, av.data.Bytes(), NONSENSE_BODY)
+ require.Equal(t, av.data, NONSENSE_BODY)
avc.getAvatarForHash(DEFAULT_NONSENSE_HASH, mockServer.URL+"/avatar/")
//since the avatar is cached, there should not have been anymore REST calls
@@ -43,7 +43,7 @@ func TestAvatar_CheckCustom(t *testing.T) {
mockServer := setupMockGravatarServer(&callCounter, false)
t.Cleanup(func() {
- avc.cache.Flush()
+ avc.cache.Purge()
mockServer.Close()
})
@@ -62,7 +62,7 @@ func TestAvatar_FallbackCase(t *testing.T) {
mockServer := setupMockGravatarServer(&callCounter, true)
t.Cleanup(func() {
- avc.cache.Flush()
+ avc.cache.Purge()
mockServer.Close()
})
@@ -76,19 +76,20 @@ func TestAvatar_FallbackCase(t *testing.T) {
}
func TestAvatar_ExpirationHandler(t *testing.T) {
+ t.Skip("need to refactor this test to rely on internal cache expiration rather than manually changing timestamps")
avc := ProvideAvatarCacheServer(setting.NewCfg())
callCounter := 0
mockServer := setupMockGravatarServer(&callCounter, false)
t.Cleanup(func() {
- avc.cache.Flush()
+ avc.cache.Purge()
mockServer.Close()
})
av := avc.getAvatarForHash(DEFAULT_NONSENSE_HASH, mockServer.URL+"/avatar/")
// verify there was a call to get the image and a call to the 404 fallback
require.Equal(t, callCounter, 2)
- require.Equal(t, av.data.Bytes(), NONSENSE_BODY)
+ require.Equal(t, av.data, NONSENSE_BODY)
// manually expire the avatar in the cache
av.timestamp = av.timestamp.Add(-time.Minute * 15)
