fix: suppress misleading upstream CPE for PHP extension binaries - #5102
Open
pujitha24 wants to merge 1 commit into
Open
fix: suppress misleading upstream CPE for PHP extension binaries#5102pujitha24 wants to merge 1 commit into
pujitha24 wants to merge 1 commit into
Conversation
Motivation: syft's PHP interpreter cataloger reports each bundled PHP C extension (openssl, ldap, sqlite3, etc.) as its own package, using the PHP interpreter's own version number as the package version (since these extensions are compiled into a specific PHP release and have no independent version of their own). The classifier already attaches an explicit CPE like cpe:2.3:a:php-openssl:php-openssl:8.5.7:... to make clear this refers to the bundled PHP extension, not upstream OpenSSL. However, that CPE was marked with Source: cpe.GeneratedSource. Syft's package-finalization pipeline (internal/task/package_task_factory.go) only skips its generic, name-based CPE generation when a package already has an "authoritative" CPE (any Source other than GeneratedSource) - so it also generated and appended a second CPE, cpe:2.3:a:openssl:openssl:8.5.7:..., derived from the bare package name. That second CPE is indistinguishable from a real standalone OpenSSL/LDAP/SQLite installation and can cause vulnerability scanners that consume this SBOM to falsely flag PHP hosts as running vulnerable upstream libraries at version "8.5.7" (a PHP version, not a real OpenSSL/LDAP/SQLite release). See anchore#5014 for a full repro. Approach: Mark the classifier's explicit CPE with Source: cpe.DeclaredSource instead of cpe.GeneratedSource, so it is treated as authoritative and the extra name-derived CPE is no longer generated. This mirrors the existing pattern in syft/pkg/cataloger/java/parse_jvm_release.go's newJvmCpe, which uses the same DeclaredSource + comment convention to mark a syft-synthesized (not literally file-declared) CPE as trustworthy and suppress further generation. It also brings this classifier in line with its sibling php-cli/php-fpm/php-apache classifiers in the same file, which already mark their CPEs as authoritative (via NVDDictionaryLookupSource, not applicable here since php-<ext> is not a real NVD dictionary entry). This change only suppresses the extra generated CPE; it does not change the package name, type, version, or PURL, and the existing php-<ext>:php-<ext> CPE is unaffected. Validation: go build ./... go test ./syft/pkg/cataloger/php/... ./internal/task/... ./syft/pkg/cataloger/internal/cpegenerate/... All pass except the two pre-existing docker-image-fixture subtests of Test_InterpreterCataloger, which fail in this sandbox only because the `docker` binary isn't available (exec: "docker": executable file not found in $PATH); confirmed via `git stash` that these two subtests fail identically on main without this change, i.e. this is a sandbox limitation and not a regression. Added a new unit test, Test_getClassifier_declaredCPESource, that directly asserts the classifier's CPE now carries Source: cpe.DeclaredSource. Fixes anchore#5014 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes false-positive/misleading CPE generation for PHP bundled extension binaries (e.g., openssl.so, ldap.so, sqlite3.so) by marking the classifier-provided php-<ext> CPE as authoritative, preventing downstream name-based CPE generation that would incorrectly imply an upstream product at the PHP version.
Changes:
- Update the PHP extension binary classifier to emit its explicit
php-<ext>CPE withSource: cpe.DeclaredSource(so finalization treats it as authoritative and skips additional name-derived CPEs). - Add a regression unit test asserting the PHP extension classifier’s CPE source is
cpe.DeclaredSource.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| syft/pkg/cataloger/php/interpreter_cataloger.go | Marks PHP extension CPEs as authoritative (DeclaredSource) to suppress misleading upstream CPE generation. |
| syft/pkg/cataloger/php/interpreter_cataloger_test.go | Adds regression coverage ensuring the classifier emits a declared (authoritative) CPE source. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Motivation:
syft's PHP interpreter cataloger reports each bundled PHP C extension
(openssl, ldap, sqlite3, etc.) as its own package, using the PHP
interpreter's own version number as the package version (since these
extensions are compiled into a specific PHP release and have no
independent version of their own). The classifier already attaches an
explicit CPE like cpe:2.3:a:php-openssl:php-openssl:8.5.7:... to make
clear this refers to the bundled PHP extension, not upstream OpenSSL.
However, that CPE was marked with Source: cpe.GeneratedSource. Syft's
package-finalization pipeline (internal/task/package_task_factory.go)
only skips its generic, name-based CPE generation when a package
already has an "authoritative" CPE (any Source other than
GeneratedSource) - so it also generated and appended a second CPE,
cpe:2.3:a:openssl:openssl:8.5.7:..., derived from the bare package
name. That second CPE is indistinguishable from a real standalone
OpenSSL/LDAP/SQLite installation and can cause vulnerability scanners
that consume this SBOM to falsely flag PHP hosts as running vulnerable
upstream libraries at version "8.5.7" (a PHP version, not a real
OpenSSL/LDAP/SQLite release). See #5014 for a full repro.
Approach:
Mark the classifier's explicit CPE with Source: cpe.DeclaredSource
instead of cpe.GeneratedSource, so it is treated as authoritative and
the extra name-derived CPE is no longer generated. This mirrors the
existing pattern in
syft/pkg/cataloger/java/parse_jvm_release.go's newJvmCpe, which uses
the same DeclaredSource + comment convention to mark a
syft-synthesized (not literally file-declared) CPE as trustworthy and
suppress further generation. It also brings this classifier in line
with its sibling php-cli/php-fpm/php-apache classifiers in the same
file, which already mark their CPEs as authoritative (via
NVDDictionaryLookupSource, not applicable here since php- is not
a real NVD dictionary entry).
This change only suppresses the extra generated CPE; it does not
change the package name, type, version, or PURL, and the existing
php-:php- CPE is unaffected.
Validation:
go build ./...
go test ./syft/pkg/cataloger/php/... ./internal/task/... ./syft/pkg/cataloger/internal/cpegenerate/...
All pass except the two pre-existing docker-image-fixture subtests of
Test_InterpreterCataloger, which fail in this sandbox only because the
dockerbinary isn't available (exec: "docker": executable file notfound in $PATH); confirmed via
git stashthat these two subtestsfail identically on main without this change, i.e. this is a sandbox
limitation and not a regression. Added a new unit test,
Test_getClassifier_declaredCPESource, that directly asserts the
classifier's CPE now carries Source: cpe.DeclaredSource.
Fixes #5014
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com