feat: remove EXIF/XMP metadata from uploaded images (#1477)

backend/internal/service/app_images_service.go

@@ -12,6 +12,7 @@ import (
 	"github.com/pocket-id/pocket-id/backend/internal/common"
 	"github.com/pocket-id/pocket-id/backend/internal/storage"
 	"github.com/pocket-id/pocket-id/backend/internal/utils"
+	imageutil "github.com/pocket-id/pocket-id/backend/internal/utils/image"
 )
 
 type AppImagesService struct {
@@ -68,7 +69,12 @@ func (s *AppImagesService) UpdateImage(ctx context.Context, file *multipart.File
 	}
 	defer fileReader.Close()
 
-	if err := s.storage.Save(ctx, imagePath, fileReader); err != nil {
+	strippedReader, err := imageutil.StripMetadata(fileReader, fileType)
+	if err != nil {
+		return err
+	}
+
+	if err := s.storage.Save(ctx, imagePath, strippedReader); err != nil {
 		return err
 	}
 

backend/internal/service/app_images_service_test.go

@@ -3,6 +3,7 @@ package service
 import (
 	"bytes"
 	"context"
+	"encoding/binary"
 	"io"
 	"io/fs"
 	"mime/multipart"
@@ -56,6 +57,29 @@ func TestAppImagesService_UpdateImage(t *testing.T) {
 	require.ErrorIs(t, err, fs.ErrNotExist)
 }
 
+func TestAppImagesService_UpdateImageStripsMetadata(t *testing.T) {
+	store, err := storage.NewFilesystemStorage(t.TempDir())
+	require.NoError(t, err)
+
+	service := NewAppImagesService(map[string]string{}, store)
+
+	fileHeader := newFileHeader(t, "logo.webp", webpFile(
+		webpChunk("VP8 ", []byte{1, 2, 3, 4}),
+		webpChunk("EXIF", []byte("secret")),
+	))
+
+	require.NoError(t, service.UpdateImage(context.Background(), fileHeader, "logoLight"))
+
+	reader, _, err := store.Open(context.Background(), path.Join("application-images", "logoLight.webp"))
+	require.NoError(t, err)
+	defer reader.Close()
+
+	payload, err := io.ReadAll(reader)
+	require.NoError(t, err)
+	assert.NotContains(t, string(payload), "secret")
+	assert.Contains(t, string(payload), "VP8 ")
+}
+
 func TestAppImagesService_ErrorsAndFlags(t *testing.T) {
 	store, err := storage.NewFilesystemStorage(t.TempDir())
 	require.NoError(t, err)
@@ -112,3 +136,26 @@ func newFileHeader(t *testing.T, filename string, content []byte) *multipart.Fil
 
 	return fileHeader
 }
+
+func webpFile(chunks ...[]byte) []byte {
+	var out bytes.Buffer
+	out.WriteString("RIFF")
+	out.Write([]byte{0, 0, 0, 0})
+	out.WriteString("WEBP")
+	for _, chunk := range chunks {
+		out.Write(chunk)
+	}
+	binary.LittleEndian.PutUint32(out.Bytes()[4:8], uint32(out.Len()-8)) //nolint:gosec
+	return out.Bytes()
+}
+
+func webpChunk(chunkType string, data []byte) []byte {
+	var out bytes.Buffer
+	out.WriteString(chunkType)
+	_ = binary.Write(&out, binary.LittleEndian, uint32(len(data))) //nolint:gosec
+	out.Write(data)
+	if len(data)%2 == 1 {
+		out.WriteByte(0)
+	}
+	return out.Bytes()
+}

backend/internal/service/oidc_service.go

@@ -1,7 +1,6 @@
 package service
 
 import (
-	"bytes"
 	"context"
 	"crypto/sha256"
 	"crypto/subtle"
@@ -35,6 +34,7 @@ import (
 	datatype "github.com/pocket-id/pocket-id/backend/internal/model/types"
 	"github.com/pocket-id/pocket-id/backend/internal/storage"
 	"github.com/pocket-id/pocket-id/backend/internal/utils"
+	imageutil "github.com/pocket-id/pocket-id/backend/internal/utils/image"
 )
 
 const (
@@ -746,11 +746,10 @@ func (s *OidcService) introspectRefreshToken(ctx context.Context, clientID strin
 		).
 		First(&storedRefreshToken).
 		Error
-	if err != nil {
-		if errors.Is(err, gorm.ErrRecordNotFound) {
-			introspectDto.Active = false
-			return introspectDto, nil
-		}
+	if errors.Is(err, gorm.ErrRecordNotFound) {
+		introspectDto.Active = false
+		return introspectDto, nil
+	} else if err != nil {
 		return introspectDto, err
 	}
 
@@ -1064,7 +1063,12 @@ func (s *OidcService) UpdateClientLogo(ctx context.Context, clientID string, fil
 		return err
 	}
 	defer reader.Close()
-	err = s.fileStorage.Save(ctx, imagePath, reader)
+	strippedReader, err := imageutil.StripMetadata(reader, fileType)
+	if err != nil {
+		return err
+	}
+
+	err = s.fileStorage.Save(ctx, imagePath, strippedReader)
 	if err != nil {
 		return err
 	}
@@ -2180,20 +2184,19 @@ func (s *OidcService) downloadAndSaveLogoFromURL(parentCtx context.Context, clie
 		darkSuffix = "-dark"
 	}
 
-	// Buffer the body so that storage backends receive a seekable reader with a known content length,
-	// which is required for correct checksum calculation on S3-compatible services
-	limitedBody := utils.NewLimitReader(resp.Body, maxLogoSize+1)
-	buf, err := io.ReadAll(limitedBody)
+	limitReader := utils.NewLimitReader(resp.Body, maxLogoSize+1)
+	strippedReader, err := imageutil.StripMetadata(limitReader, ext)
 	if errors.Is(err, utils.ErrSizeExceeded) {
-		if errors.Is(err, utils.ErrSizeExceeded) {
-			return errLogoTooLarge
-		} else if err != nil {
-			return err
-		}
+		return errLogoTooLarge
+	} else if err != nil {
+		return err
 	}
 
 	imagePath := path.Join("oidc-client-images", clientID+darkSuffix+"."+ext)
-	if err = s.fileStorage.Save(ctx, imagePath, bytes.NewReader(buf)); err != nil {
+	err = s.fileStorage.Save(ctx, imagePath, strippedReader)
+	if errors.Is(err, utils.ErrSizeExceeded) {
+		return errLogoTooLarge
+	} else if err != nil {
 		return err
 	}
 

backend/internal/utils/image/metadata.go

@@ -0,0 +1,106 @@
+package profilepicture
+
+import (
+	"bytes"
+	"encoding/binary"
+	"io"
+	"strings"
+
+	"github.com/zitadel/exifremove/pkg/exifremove"
+)
+
+const maxRIFFSize = ^uint32(0)
+
+// StripMetadata removes EXIF/XMP metadata from JPG, PNG and WEBP images
+// Returns a *bytes.Reader so that storage backends receive a seekable reader with a known content length,
+// which is required for correct checksum calculation on S3-compatible services
+func StripMetadata(file io.Reader, ext string) (*bytes.Reader, error) {
+	data, err := io.ReadAll(file)
+	if err != nil {
+		return nil, err
+	}
+
+	switch strings.ToLower(ext) {
+	case "jpg", "jpeg":
+		stripped, err := exifremove.Remove(data)
+		if err == nil {
+			return bytes.NewReader(stripped), nil
+		}
+		return bytes.NewReader(data), nil
+	case "png":
+		stripped, err := exifremove.Remove(data)
+		if err == nil {
+			return bytes.NewReader(stripped), nil
+		}
+		return bytes.NewReader(data), nil
+	case "webp":
+		return bytes.NewReader(stripWEBPMetadata(data)), nil
+	default:
+		return bytes.NewReader(data), nil
+	}
+}
+
+func stripWEBPMetadata(data []byte) []byte {
+	// Check if the file contains the RIFF...WEBP header
+	if len(data) < 12 || string(data[:4]) != "RIFF" || string(data[8:12]) != "WEBP" {
+		return data
+	}
+
+	var out bytes.Buffer
+	// Build the WEBP header
+	out.WriteString("RIFF")
+	out.Write([]byte{0, 0, 0, 0}) // Size will be filled at the end
+	out.WriteString("WEBP")
+
+	for pos := 12; pos < len(data); {
+		// Each RIFF chunk needs an 8 byte header
+		if pos+8 > len(data) {
+			return data
+		}
+
+		// Read the chunk type and payload size from the header
+		chunkType := string(data[pos : pos+4])
+		chunkSize := int(binary.LittleEndian.Uint32(data[pos+4 : pos+8]))
+		chunkEnd := pos + 8 + chunkSize
+		// Chunks with odd payload sizes include one additional byte at the end
+		if chunkSize%2 == 1 {
+			chunkEnd++
+		}
+
+		// End of chunk can't be more than the actual image data length
+		if chunkEnd > len(data) {
+			return data
+		}
+
+		// Remove chunks with the EXIF or XMP data type
+		if chunkType == "EXIF" || chunkType == "XMP " {
+			pos = chunkEnd
+			continue
+		}
+
+		// In the VP8X chunk there is a feature flag if the file contains any EXIF or XMP data
+		if chunkType == "VP8X" && chunkSize >= 10 {
+			// Copy the chunk because we don't want to modify the original one
+			chunk := make([]byte, chunkEnd-pos)
+			copy(chunk, data[pos:chunkEnd])
+
+			// Clear the Exif and XMP feature flags
+			chunk[8] &^= 0x0c
+			out.Write(chunk)
+		} else {
+			out.Write(data[pos:chunkEnd])
+		}
+
+		pos = chunkEnd
+	}
+
+	// WEBP image can max be 4GB in size
+	riffSize := out.Len() - 8
+	if riffSize < 0 || riffSize > int(maxRIFFSize) {
+		return data
+	}
+
+	// Set the size in the header (byte 4-7)
+	binary.LittleEndian.PutUint32(out.Bytes()[4:8], uint32(riffSize))
+	return out.Bytes()
+}

backend/internal/utils/image/metadata_test.go

@@ -0,0 +1,81 @@
+package profilepicture
+
+import (
+	"bytes"
+	"encoding/binary"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestStripMetadata(t *testing.T) {
+	t.Run("removes WEBP EXIF and XMP chunks", func(t *testing.T) {
+		input := webpFile(
+			webpChunk("VP8X", []byte{0x0c, 0, 0, 0, 0, 0, 0, 0, 0, 0}),
+			webpChunk("VP8 ", []byte{1, 2, 3, 4}),
+			webpChunk("EXIF", []byte("gps")),
+			webpChunk("XMP ", []byte("xmp")),
+			webpChunk("ICCP", []byte("profile")),
+		)
+
+		output := stripReader(t, input, "webp")
+
+		assert.Contains(t, string(output), "VP8 ")
+		assert.NotContains(t, string(output), "gps")
+		assert.NotContains(t, string(output), "xmp")
+		assert.Contains(t, string(output), "profile")
+		assert.Equal(t, byte(0), output[20]&0x0c)
+	})
+
+	t.Run("leaves non photo metadata formats unchanged", func(t *testing.T) {
+		input := []byte(`<svg><metadata>secret</metadata><path d="M0 0"/></svg>`)
+
+		output := stripReader(t, input, "svg")
+
+		assert.Equal(t, input, output)
+	})
+
+	t.Run("leaves malformed photo data unchanged", func(t *testing.T) {
+		input := []byte("fake-png-content")
+
+		output := stripReader(t, input, "png")
+
+		assert.Equal(t, input, output)
+	})
+}
+
+func stripReader(t *testing.T, input []byte, ext string) []byte {
+	t.Helper()
+
+	reader, err := StripMetadata(bytes.NewReader(input), ext)
+	require.NoError(t, err)
+
+	output, err := io.ReadAll(reader)
+	require.NoError(t, err)
+	return output
+}
+
+func webpFile(chunks ...[]byte) []byte {
+	var out bytes.Buffer
+	out.WriteString("RIFF")
+	out.Write([]byte{0, 0, 0, 0})
+	out.WriteString("WEBP")
+	for _, chunk := range chunks {
+		out.Write(chunk)
+	}
+	binary.LittleEndian.PutUint32(out.Bytes()[4:8], uint32(out.Len()-8)) //nolint:gosec
+	return out.Bytes()
+}
+
+func webpChunk(chunkType string, data []byte) []byte {
+	var out bytes.Buffer
+	out.WriteString(chunkType)
+	_ = binary.Write(&out, binary.LittleEndian, uint32(len(data))) //nolint:gosec
+	out.Write(data)
+	if len(data)%2 == 1 {
+		out.WriteByte(0)
+	}
+	return out.Bytes()
+}