Repository navigation
fix(storage-resize-images): bump sharp to ^0.35.5 and restrict libvips loaders to supported formats - #3247
Open
cybrdude wants to merge 4 commits into
Open
fix(storage-resize-images): bump sharp to ^0.35.5 and restrict libvips loaders to supported formats#3247cybrdude wants to merge 4 commits into
cybrdude wants to merge 4 commits into
Conversation
… bump Part 1 of 4 of the sharp ^0.35.5 bump and libvips loader allowlist (CHANGELOG entry, extension.yaml 0.3.7). Functions code, lockfile and tests follow in the next commits on this branch.
Part 2 of 4: package.json sharp ^0.35.5 and the matching lockfile (sharp 0.35.5, @img/* 0.35.5 and sharp-libvips 1.3.4, @emnapi/runtime 1.11.3; no other entries change). Brings libvips 8.18.7, libheif 1.23.5 and librsvg 2.63.2 (GHSA-f88m-g3jw-g9cj, GHSA-rgj7-g3m4-5g8c, GHSA-wq5f-xc86-pv6w).
… formats Part 3 of 4: src/loaders.ts blocks the VipsForeignLoad base class at module load and re-enables only VipsForeignLoadJpeg, Png, Tiff, Webp, Nsgif and Heif, the loaders for supportedContentTypes. resize-image.ts calls restrictLoaders() once. shouldResize() reads the uploader-set contentType while libvips picks the loader from the bytes, so without this an SVG or any other decodable file uploaded under a supported contentType still reached the loader for its real format.
Contributor
There was a problem hiding this comment.
Code Review
This pull request bumps the sharp dependency to version 0.35.5 and introduces a mechanism to restrict libvips loaders to only the supported image formats (JPEG, PNG, TIFF, WebP, GIF, and HEIF). This restriction prevents files of unsupported formats (such as SVG or PDF) from being decoded even if they are uploaded with a spoofed, supported content-type metadata. Tests and a test asset have been added to verify this behavior. There are no review comments, and I have no feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3246
shouldResize()admits an object by itscontentType, which is metadata the uploader sets, while libvips selects the loader from the file bytes. An SVG uploaded asimage/pngtherefore reaches librsvg, a format this extension does not list as supported. The published 0.3.6 bundle (sharp 0.34.5: libvips 8.17.3, libheif 1.20.2, librsvg 2.61.2) is behind three sharp advisories: GHSA-f88m-g3jw-g9cj (< 0.35.0, GIF and TIFF loaders), GHSA-rgj7-g3m4-5g8c (< 0.35.4, libheif, AVIF) and GHSA-wq5f-xc86-pv6w (< 0.35.5, librsvg, SVG).nextat 0.35.3 still carries the last two.This PR:
sharpto^0.35.5(libvips 8.18.7, libheif 1.23.5, librsvg 2.63.2 in the prebuilt binaries); the lockfile diff is sharp, its@img/*platform packages and@emnapi/runtime;src/loaders.ts: block theVipsForeignLoadbase class at module load and re-enable only the loaders forsupportedContentTypes(VipsForeignLoadJpeg,VipsForeignLoadPng,VipsForeignLoadTiff,VipsForeignLoadWebp,VipsForeignLoadNsgif,VipsForeignLoadHeif), called once fromresize-image.ts;__tests__/loaders.test.ts: the contentType gate alone admits SVG bytes labelledimage/png;resize()now rejects them with "unsupported image format"; jpeg, png, tiff, webp, gif and avif still resize. The second test fails on the current code.Tests:
jest __tests__/loaders.test.ts __tests__/resize.test.ts __tests__/convert-image.test.ts __tests__/filters.test.ts __tests__/util.test.ts-> 30 passed.__tests__/unit/*fail identically before and after (they expect config env the suite does not set).Reported through the channels in SECURITY.md as well (Google VRP b/574386863, advisory GHSA-pvx3-hg35-5vwm); this PR is the fix side.