Incomplete trailing escape (%) pattern #6688

acanda · 2024-05-25T16:36:18Z

Describe the bug
The dependency-check throws an IllegalArgumentException URLDecoder: Incomplete trailing escape (%) pattern when analyzing the dependency org.scalameta:common_2.13:4.9.1. The actual culprit seems to be the ~~transitive~~ dependency com.google.summit:summit-ast:2.2.0 with following two softwareIdentifiers:

pkg:maven/com.google.summit/summit-ast@2.2.0
pkg:maven/com.google.summit/summit-ast@2.2.0%A

The second identifier is not a valid URL, leading to URLDecoder failing with Incomplete trailing escape (%) pattern.

Version of dependency-check used
The problem occurs using version 9.2.0 of the maven plugin. It does not occur with earlier versions.

Log file

Warning:  An error occurred while analyzing '/home/runner/.m2/repository/org/scalameta/common_2.13/4.9.1/common_2.13-4.9.1.jar' (Sonatype OSS Index Analyzer).
[DEBUG] 
org.owasp.dependencycheck.analyzer.exception.AnalysisException: Failed to request component-reports
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.analyzeDependency (OssIndexAnalyzer.java:170)
    at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze (AbstractAnalyzer.java:131)
    at org.owasp.dependencycheck.AnalysisTask.call (AnalysisTask.java:88)
    at org.owasp.dependencycheck.AnalysisTask.call (AnalysisTask.java:37)
    at java.util.concurrent.FutureTask.run (FutureTask.java:264)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1136)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:635)
    at java.lang.Thread.run (Thread.java:840)
Caused by: java.lang.IllegalArgumentException: URLDecoder: Incomplete trailing escape (%) pattern
    at java.net.URLDecoder.decode (URLDecoder.java:230)
    at java.net.URLDecoder.decode (URLDecoder.java:147)
    at org.sonatype.goodies.packageurl.PercentEncoding.decode (PercentEncoding.java:78)
    at org.sonatype.goodies.packageurl.PackageUrlParser.parseVersion (PackageUrlParser.java:144)
    at org.sonatype.goodies.packageurl.PackageUrlParser.parse (PackageUrlParser.java:107)
    at org.sonatype.goodies.packageurl.PackageUrl.parse (PackageUrl.java:293)
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.parsePackageUrl (OssIndexAnalyzer.java:203)
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.lambda$null$1 (OssIndexAnalyzer.java:223)
    at java.util.stream.ReferencePipeline$3$1.accept (ReferencePipeline.java:197)
    at java.util.stream.ReferencePipeline$2$1.accept (ReferencePipeline.java:179)
    at java.util.TreeMap$KeySpliterator.forEachRemaining (TreeMap.java:3064)
    at java.util.stream.AbstractPipeline.copyInto (AbstractPipeline.java:509)
    at java.util.stream.AbstractPipeline.wrapAndCopyInto (AbstractPipeline.java:499)
    at java.util.stream.ForEachOps$ForEachOp.evaluateSequential (ForEachOps.java:150)
    at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential (ForEachOps.java:173)
    at java.util.stream.AbstractPipeline.evaluate (AbstractPipeline.java:234)
    at java.util.stream.ReferencePipeline.forEach (ReferencePipeline.java:596)
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.lambda$requestReports$3 (OssIndexAnalyzer.java:225)
    at java.util.Spliterators$ArraySpliterator.forEachRemaining (Spliterators.java:992)
    at java.util.stream.ReferencePipeline$Head.forEach (ReferencePipeline.java:762)
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.requestReports (OssIndexAnalyzer.java:221)
    at org.owasp.dependencycheck.analyzer.OssIndexAnalyzer.analyzeDependency (OssIndexAnalyzer.java:136)
    at org.owasp.dependencycheck.analyzer.AbstractAnalyzer.analyze (AbstractAnalyzer.java:131)
    at org.owasp.dependencycheck.AnalysisTask.call (AnalysisTask.java:88)
    at org.owasp.dependencycheck.AnalysisTask.call (AnalysisTask.java:37)
    at java.util.concurrent.FutureTask.run (FutureTask.java:264)
    at java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1136)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run (ThreadPoolExecutor.java:635)
    at java.lang.Thread.run (Thread.java:840)

Full log: https://github.com/acanda/code-analysis-maven-plugin/actions/runs/9236637535/job/25412701360?pr=289

To Reproduce
Steps to reproduce the behavior:

Clone https://github.com/acanda/code-analysis-maven-plugin
Checkout branch renovate/org.owasp-dependency-check-maven-9.x
Set the environment variable NVD_API_KEY to your NVD API key
Run ./mvnw dependency-check:check
See error

Expected behavior
The dependency check should not throw an AnalysisException and finish the dependency check successfully.

The text was updated successfully, but these errors were encountered:

nhumblot · 2024-05-27T05:49:11Z

Hi! 👋

Thank you for reaching us.

OssIndexAnalyzer.parsePackageUrl() is catching a org.sonatype.goodies.packageurl.InvalidException it is not including this IllegalArgumentException, which does not seems to be controlled properly by the InvalidException class. By reading the JavaDoc, I would expect this exception to be raised: Thrown when package-url is detected to be invalid for some reason..

Changing the current implementation of OssIndexAnalyzer.parsePackageUrl() to catch a RuntimeException prevents the analysis to crash (but only alert of an invalid package at debug level):

    /**
     * Helper to complain if unable to parse Package-URL.
     *
     * @param value the url to parse
     * @return the package url
     */
    @Nullable
    private PackageUrl parsePackageUrl(final String value) {
        try {
            return PackageUrl.parse(value);
        } catch (RuntimeException e) {
            LOG.debug("Invalid Package-URL: {}", value, e);
            return null;
        }
    }

(see: 6f76587)

We get the value from com.github.packageurl.PackageURL.canonicalize(), returning us the pkg:maven/com.google.summit/summit-ast@2.2.0%A value we then try to parse. The purl-spec states:

A purl is a valid URL and URI that conforms to the URL definitions or specifications at:

https://tools.ietf.org/html/rfc3986

https://en.wikipedia.org/wiki/URL#Syntax

https://en.wikipedia.org/wiki/Uniform_Resource_Identifier#Syntax

https://url.spec.whatwg.org/

% is a character that must be encoded in a URL. I think we could reach https://github.com/package-url/packageurl-java a open a bug regarding this. I will do it and ask for their opinion. I would have expected PackageURL to throw an IllegalArgumentException when getting such a value. 🤔

I don't have more time now, I will look to continue this investigation later today and see where we are getting this value as a software identifier for this dependency to make sure there may not be a deeper issue in DependencyCheck.

Sorry for the inconvenience.

jeremylong · 2024-05-27T10:13:42Z

I'm unable to reproduce this?

https://github.com/jeremylong/odc-falsepositives/blob/f1faf74b39e4df77419bdb0111bd20a6f095cf8c/pom.xml#L126-L130

I don't get the invalid PURL and when I run mvn dependency:tree I see:

[INFO] com.mycompany:odc-falsepositives:jar:1.1-SNAPSHOT
[INFO] +- org.scalameta:common_2.13:jar:4.9.1:compile
[INFO] |  +- org.scala-lang:scala-library:jar:2.13.13:compile
[INFO] |  +- com.lihaoyi:sourcecode_2.13:jar:0.3.1:compile
[INFO] |  \- com.thesamet.scalapb:scalapb-runtime_2.13:jar:0.11.15:compile
[INFO] |     +- com.thesamet.scalapb:lenses_2.13:jar:0.11.15:compile
[INFO] |     +- com.google.protobuf:protobuf-java:jar:3.19.6:compile
[INFO] |     \- org.scala-lang.modules:scala-collection-compat_2.13:jar:2.11.0:compile
[INFO] +- org.junit.jupiter:junit-jupiter-api:jar:5.6.0:test
[INFO] |  +- org.apiguardian:apiguardian-api:jar:1.1.0:test
[INFO] |  +- org.opentest4j:opentest4j:jar:1.2.0:test
[INFO] |  \- org.junit.platform:junit-platform-commons:jar:1.6.0:test
[INFO] +- org.junit.jupiter:junit-jupiter-params:jar:5.6.0:test
[INFO] \- org.junit.jupiter:junit-jupiter-engine:jar:5.6.0:test
[INFO]    \- org.junit.platform:junit-platform-engine:jar:1.6.0:test

Which does not include pkg:maven/com.google.summit/summit-ast@2.2.0. Am I missing something?

I was trying to look at where the package URL is being created as we can likely fix the bug there. But without a reproduce I can't help.

acanda · 2024-05-27T11:00:40Z

It looks like adding org.scalameta:common_2.13:4.9.1 all by itself does not trigger this behaviour. If you follow the steps I wrote down in the description you should be able to reproduce the issue.

Running mvn dependency:tree on the project linked in the description gives the following result:

[INFO] --- dependency:3.6.1:tree (default-cli) @ code-analysis-maven-plugin ---
[INFO] ch.acanda.maven:code-analysis-maven-plugin:maven-plugin:1.7.0-SNAPSHOT
[INFO] |  ...
[INFO] +- net.sourceforge.pmd:pmd-dist:jar:7.1.0:compile
[INFO] |  +- net.sourceforge.pmd:pmd-languages-deps:pom:7.1.0:runtime
[INFO] |  |  +- net.sourceforge.pmd:pmd-apex:jar:7.1.0:runtime
[INFO] |  |  |  | ...
[INFO] |  |  |  +- com.google.summit:summit-ast:jar:2.2.0:runtime
[INFO] |  |  +- net.sourceforge.pmd:pmd-scala_2.13:jar:7.1.0:runtime
[INFO] |  |  |  +- org.scala-lang:scala-library:jar:2.13.13:runtime
[INFO] |  |  |  +- org.scalameta:parsers_2.13:jar:4.9.1:runtime
[INFO] |  |  |  \- org.scalameta:trees_2.13:jar:4.9.1:runtime
[INFO] |  |  |     \- org.scalameta:common_2.13:jar:4.9.1:runtime
...

It looks like com.google.summit:summit-ast is not a direct dependency of org.scalameta:common_2.13 and summit-ast's version also does not include the %A.

I found summit-ast by debugging mvn clean verify and setting a break point when an IllegalArgumentExcepion with the condition "URLDecoder: Incomplete trailing escape (%) pattern".equals(getMessage()) is thrown.

This lead me to the following:

I don't know from where the second softwareIdentifier with the suffix %A is coming.

nhumblot · 2024-05-30T01:05:37Z

We somehow parse a pom that gets a version number with a newline in it:

When we analyze the dependency we extract the pom.xml from the jar file:

When I unzip the jar and give a look into the extracted pom.xml, there is a new line in both the <tag> and <version> XML tags.

This eventually generates the %A. The URL encoding of a new line is %0A: https://stackoverflow.com/a/3871762

We don't have the 0 prefix because it seems to be due to an implementation issue in the com.github.packageurl.PackageURL.uriEncode(String, Charset) static method. The

'\n' char is equal to 10 when casted to an int. The following instruction

builder.append(Integer.toHexString(b).toUpperCase());

does not guarantee to have a 0 prefix for int values from 10 to 15 (0 to 9 values are not executed with this instruction), making it an invalid URL.

Technically, the Maven pom schema definition states that the <version> tag is of type xs:string, which allow new lines and carriage returns, so it is a valid value according to the XML schema but it should be considered an issue IMO as there is no https://mvnrepository.com/artifact/com.google.summit/summit-ast/2.2.0%0A version 🙂

I will look to reach both projects to let them know about this. Given these observations, I am not sure we should implement a specific fix in DependencyCheck as there may actually be cases where this could be a correct value.

I honestly discovered today that it is possible to package a jar with a newline in the version 😮

If @jeremylong prefers to also have a patch in DependencyCheck to fix it faster, I can open a PR that would sanitize the version number by removing \n characters out of it before when creating the PurlIdentifier. I prefer to get your opinion as it seems hacky given what I understood from this investigation.

jeremylong · 2024-05-30T09:37:36Z

@nhumblot yes - we should trim the version number in dependency-check. Thank you for investigating this.

acanda · 2024-06-01T15:12:41Z

Thank you, I appreciate all you've done.

acanda added the bug label May 25, 2024

nhumblot self-assigned this May 27, 2024

nhumblot added a commit that referenced this issue May 27, 2024

fix: #6688 prevent IllegalArgumentException when Invalid URL is parsed

6f76587

This was referenced May 30, 2024

Instanciating a PackagerURL with a version number containing a new line leads eventually to an invalid URL package-url/packageurl-java#150

Open

2.2.0 Jar seems to include an incoherent version number in its pom google/summit-ast#57

Closed

nhumblot added the dependencies label May 30, 2024

nhumblot added a commit that referenced this issue Jun 1, 2024

fix: #6688 Trim version number when parsin POM

a70e6e3

nhumblot mentioned this issue Jun 1, 2024

fix: #6688 Trim version number when parsin POM #6705

Merged

jeremylong closed this as completed in #6705 Jun 1, 2024

jeremylong pushed a commit that referenced this issue Jun 1, 2024

fix: #6688 Trim version number when parsin POM (#6705)

db93fe4

nhumblot mentioned this issue Jun 2, 2024

version_file doesn't trim the version typedb/bazel-distribution#380

Closed

acanda mentioned this issue Jun 28, 2024

Update dependency org.owasp:dependency-check-maven to v9.2.0 acanda/code-analysis-maven-plugin#289

Closed

1 task

github-actions bot locked as resolved and limited conversation to collaborators Dec 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incomplete trailing escape (%) pattern #6688

Incomplete trailing escape (%) pattern #6688

acanda commented May 25, 2024 •

edited

Loading

nhumblot commented May 27, 2024

jeremylong commented May 27, 2024

acanda commented May 27, 2024 •

edited

Loading

nhumblot commented May 30, 2024 •

edited

Loading

jeremylong commented May 30, 2024

acanda commented Jun 1, 2024

Incomplete trailing escape (%) pattern #6688

Incomplete trailing escape (%) pattern #6688

Comments

acanda commented May 25, 2024 • edited Loading

nhumblot commented May 27, 2024

jeremylong commented May 27, 2024

acanda commented May 27, 2024 • edited Loading

nhumblot commented May 30, 2024 • edited Loading

jeremylong commented May 30, 2024

acanda commented Jun 1, 2024

acanda commented May 25, 2024 •

edited

Loading

acanda commented May 27, 2024 •

edited

Loading

nhumblot commented May 30, 2024 •

edited

Loading