fix(graphics): harden harness image validation
- commit
- df3b2f325cea94ea8e345a80260439b278b18b6e
- author
- brice · · 2026-08-24 10:30 UTC
- parent
- 6ac336a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +35 -4
M cmd/graphics-harness/main.go +11 -2
| @@ -58,10 +58,19 @@ func scaleFrame(source *image.RGBA, scale int) (*image.RGBA, error) { | |||
| 58 | 58 | return nil, fmt.Errorf("scale must be positive") | |
| 59 | 59 | } | |
| 60 | 60 | width, height := source.Bounds().Dx(), source.Bounds().Dy() | |
| 61 | - | if uint64(width)*uint64(height)*uint64(scale)*uint64(scale) > 64<<20 { | |
| 61 | + | maxInt := int(^uint(0) >> 1) | |
| 62 | + | if width > 0 && scale > maxInt/width { | |
| 62 | 63 | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | |
| 63 | 64 | } | |
| 64 | - | result := image.NewRGBA(image.Rect(0, 0, width*scale, height*scale)) | |
| 65 | + | if height > 0 && scale > maxInt/height { | |
| 66 | + | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | |
| 67 | + | } | |
| 68 | + | scaledWidth, scaledHeight := width*scale, height*scale | |
| 69 | + | const maxScaledPixels = uint64(64 << 20) | |
| 70 | + | if scaledWidth > 0 && uint64(scaledHeight) > maxScaledPixels/uint64(scaledWidth) { | |
| 71 | + | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | |
| 72 | + | } | |
| 73 | + | result := image.NewRGBA(image.Rect(0, 0, scaledWidth, scaledHeight)) | |
| 65 | 74 | for y := range height { | |
| 66 | 75 | for x := range width { | |
| 67 | 76 | pixel := source.RGBAAt(x, y) | |
| @@ -58,10 +58,19 @@ func scaleFrame(source *image.RGBA, scale int) (*image.RGBA, error) { | |||||
| 58 | return nil, fmt.Errorf("scale must be positive") | 58 | return nil, fmt.Errorf("scale must be positive") | ||
| 59 | } | 59 | } | ||
| 60 | width, height := source.Bounds().Dx(), source.Bounds().Dy() | 60 | width, height := source.Bounds().Dx(), source.Bounds().Dy() | ||
| 61 | - | if uint64(width)*uint64(height)*uint64(scale)*uint64(scale) > 64<<20 { | 61 | + | maxInt := int(^uint(0) >> 1) |
| 62 | + | if width > 0 && scale > maxInt/width { | |||
| 62 | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | 63 | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | ||
| 63 | } | 64 | } | ||
| 64 | - | result := image.NewRGBA(image.Rect(0, 0, width*scale, height*scale)) | 65 | + | if height > 0 && scale > maxInt/height { |
| 66 | + | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | |||
| 67 | + | } | |||
| 68 | + | scaledWidth, scaledHeight := width*scale, height*scale | |||
| 69 | + | const maxScaledPixels = uint64(64 << 20) | |||
| 70 | + | if scaledWidth > 0 && uint64(scaledHeight) > maxScaledPixels/uint64(scaledWidth) { | |||
| 71 | + | return nil, fmt.Errorf("scaled frame exceeds 67108864 pixels") | |||
| 72 | + | } | |||
| 73 | + | result := image.NewRGBA(image.Rect(0, 0, scaledWidth, scaledHeight)) | |||
| 65 | for y := range height { | 74 | for y := range height { | ||
| 66 | for x := range width { | 75 | for x := range width { | ||
| 67 | pixel := source.RGBAAt(x, y) | 76 | pixel := source.RGBAAt(x, y) | ||
M cmd/graphics-harness/main_test.go +5 -0
| @@ -26,3 +26,8 @@ func TestScaleFrameRejectsInvalidScale(t *testing.T) { | |||
| 26 | 26 | _, err := scaleFrame(image.NewRGBA(image.Rect(0, 0, 1, 1)), 0) | |
| 27 | 27 | require.Error(t, err) | |
| 28 | 28 | } | |
| 29 | + | ||
| 30 | + | func TestScaleFrameRejectsMaximumIntScale(t *testing.T) { | |
| 31 | + | _, err := scaleFrame(image.NewRGBA(image.Rect(0, 0, 1, 1)), int(^uint(0)>>1)) | |
| 32 | + | require.EqualError(t, err, "scaled frame exceeds 67108864 pixels") | |
| 33 | + | } | |
| @@ -26,3 +26,8 @@ func TestScaleFrameRejectsInvalidScale(t *testing.T) { | |||||
| 26 | _, err := scaleFrame(image.NewRGBA(image.Rect(0, 0, 1, 1)), 0) | 26 | _, err := scaleFrame(image.NewRGBA(image.Rect(0, 0, 1, 1)), 0) | ||
| 27 | require.Error(t, err) | 27 | require.Error(t, err) | ||
| 28 | } | 28 | } | ||
| 29 | + | ||||
| 30 | + | func TestScaleFrameRejectsMaximumIntScale(t *testing.T) { | |||
| 31 | + | _, err := scaleFrame(image.NewRGBA(image.Rect(0, 0, 1, 1)), int(^uint(0)>>1)) | |||
| 32 | + | require.EqualError(t, err, "scaled frame exceeds 67108864 pixels") | |||
| 33 | + | } | |||
M internal/graphicsharness/render.go +6 -2
| @@ -75,13 +75,17 @@ func decodeAsset(asset graphics.AssetView) (*image.RGBA, error) { | |||
| 75 | 75 | encoded := asset.Encoded() | |
| 76 | 76 | switch asset.Format() { | |
| 77 | 77 | case graphics.AssetFormatPNG: | |
| 78 | - | decoded, err := png.Decode(bytes.NewReader(encoded)) | |
| 78 | + | config, err := png.DecodeConfig(bytes.NewReader(encoded)) | |
| 79 | 79 | if err != nil { | |
| 80 | 80 | return nil, fmt.Errorf("decode asset %s: %w", asset.ID(), err) | |
| 81 | 81 | } | |
| 82 | - | if decoded.Bounds().Dx() != width || decoded.Bounds().Dy() != height { | |
| 82 | + | if config.Width != width || config.Height != height { | |
| 83 | 83 | return nil, fmt.Errorf("decode asset %s: PNG dimensions do not match snapshot", asset.ID()) | |
| 84 | 84 | } | |
| 85 | + | decoded, err := png.Decode(bytes.NewReader(encoded)) | |
| 86 | + | if err != nil { | |
| 87 | + | return nil, fmt.Errorf("decode asset %s: %w", asset.ID(), err) | |
| 88 | + | } | |
| 85 | 89 | result := image.NewRGBA(image.Rect(0, 0, width, height)) | |
| 86 | 90 | for y := range height { | |
| 87 | 91 | for x := range width { | |
| @@ -75,13 +75,17 @@ func decodeAsset(asset graphics.AssetView) (*image.RGBA, error) { | |||||
| 75 | encoded := asset.Encoded() | 75 | encoded := asset.Encoded() | ||
| 76 | switch asset.Format() { | 76 | switch asset.Format() { | ||
| 77 | case graphics.AssetFormatPNG: | 77 | case graphics.AssetFormatPNG: | ||
| 78 | - | decoded, err := png.Decode(bytes.NewReader(encoded)) | 78 | + | config, err := png.DecodeConfig(bytes.NewReader(encoded)) |
| 79 | if err != nil { | 79 | if err != nil { | ||
| 80 | return nil, fmt.Errorf("decode asset %s: %w", asset.ID(), err) | 80 | return nil, fmt.Errorf("decode asset %s: %w", asset.ID(), err) | ||
| 81 | } | 81 | } | ||
| 82 | - | if decoded.Bounds().Dx() != width || decoded.Bounds().Dy() != height { | 82 | + | if config.Width != width || config.Height != height { |
| 83 | return nil, fmt.Errorf("decode asset %s: PNG dimensions do not match snapshot", asset.ID()) | 83 | return nil, fmt.Errorf("decode asset %s: PNG dimensions do not match snapshot", asset.ID()) | ||
| 84 | } | 84 | } | ||
| 85 | + | decoded, err := png.Decode(bytes.NewReader(encoded)) | |||
| 86 | + | if err != nil { | |||
| 87 | + | return nil, fmt.Errorf("decode asset %s: %w", asset.ID(), err) | |||
| 88 | + | } | |||
| 85 | result := image.NewRGBA(image.Rect(0, 0, width, height)) | 89 | result := image.NewRGBA(image.Rect(0, 0, width, height)) | ||
| 86 | for y := range height { | 90 | for y := range height { | ||
| 87 | for x := range width { | 91 | for x := range width { | ||
M internal/graphicsharness/render_test.go +13 -0
| @@ -56,6 +56,19 @@ func TestRenderDecodesMetadataBearingPNG(t *testing.T) { | |||
| 56 | 56 | require.Equal(t, 32, frame.Bounds().Dy()) | |
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | + | func TestRenderRejectsPNGDimensionsDifferentFromSnapshot(t *testing.T) { | |
| 60 | + | encoded, err := base64.StdEncoding.DecodeString(pngSuiteEXIFBase64) | |
| 61 | + | require.NoError(t, err) | |
| 62 | + | scene := graphics.NewScene(graphics.Limits{}) | |
| 63 | + | assetID, err := scene.AddAsset(graphics.AssetBlob{Format: graphics.AssetFormatPNG, Width: 31, Height: 32, Encoded: encoded}) | |
| 64 | + | require.NoError(t, err) | |
| 65 | + | _, err = scene.Place(graphics.PlacementSpec{Asset: assetID, Destination: graphics.PixelRect{Width: 31, Height: 32}}) | |
| 66 | + | require.NoError(t, err) | |
| 67 | + | ||
| 68 | + | _, err = Render(scene.Snapshot(), graphics.PixelRect{Width: 31, Height: 32}) | |
| 69 | + | require.EqualError(t, err, "decode asset a1: PNG dimensions do not match snapshot") | |
| 70 | + | } | |
| 71 | + | ||
| 59 | 72 | func TestRenderScalesCropsAndClips(t *testing.T) { | |
| 60 | 73 | scene := graphics.NewScene(graphics.Limits{}) | |
| 61 | 74 | assetID, err := scene.AddAsset(graphics.AssetBlob{ | |
| @@ -56,6 +56,19 @@ func TestRenderDecodesMetadataBearingPNG(t *testing.T) { | |||||
| 56 | require.Equal(t, 32, frame.Bounds().Dy()) | 56 | require.Equal(t, 32, frame.Bounds().Dy()) | ||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | + | func TestRenderRejectsPNGDimensionsDifferentFromSnapshot(t *testing.T) { | |||
| 60 | + | encoded, err := base64.StdEncoding.DecodeString(pngSuiteEXIFBase64) | |||
| 61 | + | require.NoError(t, err) | |||
| 62 | + | scene := graphics.NewScene(graphics.Limits{}) | |||
| 63 | + | assetID, err := scene.AddAsset(graphics.AssetBlob{Format: graphics.AssetFormatPNG, Width: 31, Height: 32, Encoded: encoded}) | |||
| 64 | + | require.NoError(t, err) | |||
| 65 | + | _, err = scene.Place(graphics.PlacementSpec{Asset: assetID, Destination: graphics.PixelRect{Width: 31, Height: 32}}) | |||
| 66 | + | require.NoError(t, err) | |||
| 67 | + | ||||
| 68 | + | _, err = Render(scene.Snapshot(), graphics.PixelRect{Width: 31, Height: 32}) | |||
| 69 | + | require.EqualError(t, err, "decode asset a1: PNG dimensions do not match snapshot") | |||
| 70 | + | } | |||
| 71 | + | ||||
| 59 | func TestRenderScalesCropsAndClips(t *testing.T) { | 72 | func TestRenderScalesCropsAndClips(t *testing.T) { | ||
| 60 | scene := graphics.NewScene(graphics.Limits{}) | 73 | scene := graphics.NewScene(graphics.Limits{}) | ||
| 61 | assetID, err := scene.AddAsset(graphics.AssetBlob{ | 74 | assetID, err := scene.AddAsset(graphics.AssetBlob{ | ||