fix(java): read artifact properties only from the MANIFEST.MF main section - #11066
Conversation
| // parseManifestMainSection reads the main section of a MANIFEST.MF, which is the one describing the archive itself. | ||
| // It ends at the first empty line; the individual sections that may follow describe single files inside the archive, | ||
| // so their attributes say nothing about the artifact the archive ships. | ||
| // cf. https://docs.oracle.com/en/java/javase/21/docs/specs/jar/jar.html#jar-manifest | ||
| func parseManifestMainSection(r io.Reader) (manifest, error) { | ||
| var m manifest | ||
| scanner := bufio.NewScanner(file) | ||
| scanner := bufio.NewScanner(r) |
There was a problem hiding this comment.
The JAR manifest grammar permits CRLF, LF, and a lone CR as newline sequences, while bufio.ScanLines recognizes only \r?\n. I don't know how common CR-only manifests are in practice, so I don't consider this blocking, but it seems safer to support the complete grammar while introducing the main-section boundary. A custom split function can preserve streaming and wait for another byte when CR is at the end of the current buffer.
| // parseManifestMainSection reads the main section of a MANIFEST.MF, which is the one describing the archive itself. | |
| // It ends at the first empty line; the individual sections that may follow describe single files inside the archive, | |
| // so their attributes say nothing about the artifact the archive ships. | |
| // cf. https://docs.oracle.com/en/java/javase/21/docs/specs/jar/jar.html#jar-manifest | |
| func parseManifestMainSection(r io.Reader) (manifest, error) { | |
| var m manifest | |
| scanner := bufio.NewScanner(file) | |
| scanner := bufio.NewScanner(r) | |
| func scanManifestLines(data []byte, atEOF bool) (advance int, token []byte, err error) { | |
| if atEOF && len(data) == 0 { | |
| return 0, nil, nil | |
| } | |
| for i, b := range data { | |
| switch b { | |
| case '\n': | |
| return i + 1, data[:i], nil | |
| case '\r': | |
| if i+1 == len(data) && !atEOF { | |
| return 0, nil, nil | |
| } | |
| advance = i + 1 | |
| if i+1 < len(data) && data[i+1] == '\n' { | |
| advance++ | |
| } | |
| return advance, data[:i], nil | |
| } | |
| } | |
| if atEOF { | |
| return len(data), data, nil | |
| } | |
| return 0, nil, nil | |
| } | |
| // parseManifestMainSection reads the main section of a MANIFEST.MF, which is the one describing the archive itself. | |
| // It ends at the first empty line; the individual sections that may follow describe packages or files inside the archive, | |
| // so their attributes must not be used as archive-level artifact properties. | |
| // cf. https://docs.oracle.com/en/java/javase/21/docs/specs/jar/jar.html#jar-manifest | |
| func parseManifestMainSection(r io.Reader) (manifest, error) { | |
| var m manifest | |
| scanner := bufio.NewScanner(r) | |
| scanner.Split(scanManifestLines) |
Could we also add a CR-only case alongside the existing CRLF test?
|
|
||
| ## JAR/WAR/PAR/EAR | ||
| To find information about your JAR[^2] file, Trivy parses `pom.properties` and `MANIFEST.MF` files in your JAR[^2] file and takes required properties[^3]. | ||
| Only the main section of `MANIFEST.MF` is used, because the individual sections that may follow it describe single files inside the archive rather than the archive itself. |
There was a problem hiding this comment.
nit: The JAR specification says that individual sections can describe packages or files. Their Implementation-* and Specification-* attributes are valid scoped metadata, although they shouldn't be used as archive-level artifact properties.
| Only the main section of `MANIFEST.MF` is used, because the individual sections that may follow it describe single files inside the archive rather than the archive itself. | |
| Only the main section of `MANIFEST.MF` is used for artifact properties, because the individual sections that may follow it describe packages or files inside the archive rather than the archive itself. |
Description
A
MANIFEST.MFstarts with a main section describing the archive itself, and may continue with individual sections, each introduced by aName:attribute and describing a single file or package inside the archive.The two are separated by an empty line.
parseManifestread the whole file, soImplementation-*,Specification-*andBundle-*attributes from individual sections overwrote the values of the main section — and since assignment is unconditional, the last section in the file won.The artifact was then identified by attributes that describe a package shipped inside the JAR rather than the JAR itself.
This PR stops the parsing at the empty line that ends the main section, as the JAR specification defines it.
The loop body is unchanged; it moved into
parseManifestMainSection, which takes anio.Readerso the section handling can be unit-tested without building a zip.Since the section boundary is now a line, the lines have to be split the way the manifest grammar defines them: it allows CRLF, LF and a lone CR, while
bufio.ScanLinesrecognises only the first two.scanManifestLinescovers all three and stays streaming — when a CR is the last byte of the buffer it waits for the next byte instead of guessing whether a LF follows.Before / after
Measured on 20 popular artifacts from Maven Central.
17 of them are unaffected: their attributes are in the main section, exactly where the parser now looks.
The three that change are the ones that keep artifact attributes in individual sections:
xercesImpl-2.12.2.jarxerces:xercesImpl2.12.2xercesImpl-2.12.2.jarApache Software Foundation:org.apache.xerces.xni1.2ant-1.10.15.jarorg.apache.ant:ant1.10.15ant-1.10.15.jarApache Software Foundation:org.apache.tools.ant1.10.15log4j-1.2.17.jarlog4j:log4j1.2.17pom.propertiesis preferred over the manifest)Nothing changes online because
resolveArtifactonly trusts manifest properties onceclient.Exists(groupID, artifactID)confirms them: neitherApache Software Foundation:org.apache.xerces.xninorApache Software Foundation:org.apache.tools.antis a real Maven coordinate, so the lookup by SHA-1 already took over and returned the correct artifact.Offline the wrong values used to be reported as they were, and such a JAR is now left unidentified instead.
That is the intended trade-off: a name that matches no artifact in any repository cannot match a vulnerability either, so what disappears from the report is a fabricated component — carrying, in the
xercesImplcase, the version of an inner package (1.2) instead of the version of the archive (2.12.2).Jenkins
Plugin-License-Nameattributes are main-section attributes and keep working.Tests
TestManifestPropertiescovers the main section alone, an individual section that must not override it, artifact attributes present only in an individual section, and CRLF and CR-only line endings.Four of the five cases fail on current
main.Related issues
Split out of #10948, where it came up in review.
Checklist