From eb7053f48c41dfe81902575ef5d261340ffd2d64 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 28 May 2021 16:00:37 +0100 Subject: [PATCH 1/6] Add classes representing Maven shade relocations; use them to recognise a shaded version of SnakeYaml --- .../semmle/code/java/frameworks/SnakeYaml.qll | 8 ++- java/ql/src/semmle/code/xml/MavenPom.qll | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll b/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll index db5687f916ab..a029ce5d1611 100644 --- a/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll +++ b/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll @@ -6,13 +6,14 @@ import java import semmle.code.java.dataflow.DataFlow import semmle.code.java.dataflow.DataFlow2 import semmle.code.java.dataflow.DataFlow3 +import semmle.code.xml.MavenPom /** * The class `org.yaml.snakeyaml.constructor.SafeConstructor`. */ class SnakeYamlSafeConstructor extends RefType { SnakeYamlSafeConstructor() { - this.hasQualifiedName("org.yaml.snakeyaml.constructor", "SafeConstructor") + this.hasQualifiedName(getAShadedPackage("org.yaml.snakeyaml.constructor"), "SafeConstructor") } } @@ -27,7 +28,10 @@ class SafeSnakeYamlConstruction extends ClassInstanceExpr { * The class `org.yaml.snakeyaml.Yaml`. */ class Yaml extends RefType { - Yaml() { this.getASupertype*().hasQualifiedName("org.yaml.snakeyaml", "Yaml") } + Yaml() { + this.getASupertype*() + .hasQualifiedName(getAShadedPackage("org.yaml.snakeyaml.constructor"), "Yaml") + } } private class SafeYamlConstructionFlowConfig extends DataFlow2::Configuration { diff --git a/java/ql/src/semmle/code/xml/MavenPom.qll b/java/ql/src/semmle/code/xml/MavenPom.qll index 7619be3293a0..56c38dfd6ec9 100644 --- a/java/ql/src/semmle/code/xml/MavenPom.qll +++ b/java/ql/src/semmle/code/xml/MavenPom.qll @@ -485,3 +485,54 @@ class MavenRepoJar extends File { else getVersion().matches(pom.getVersionString() + "%") } } + +class MavenPlugin extends ProtoPom { + MavenPlugin() { this.hasName("plugin") } +} + +class MavenShadePlugin extends MavenPlugin { + MavenShadePlugin() { + this.getGroup().getValue() = "org.apache.maven.plugins" and + this.getArtifact().getValue() = "maven-shade-plugin" + } + + MavenShadeRelocation getARelocation() { result.getPlugin() = this } +} + +class MavenShadeRelocation extends XMLElement { + MavenShadePlugin plugin; + + MavenShadeRelocation() { + this.getParent().(XMLElement).getParent().(XMLElement).getParent() = plugin and + this.hasName("relocation") + } + + MavenShadePlugin getPlugin() { result = plugin } + + string getPattern() { + exists(XMLElement el | el.getName() = "pattern" and el.getParent() = this | + result = el.getTextValue() + ) + } + + string getShadedPattern() { + exists(XMLElement el | el.getName() = "shadedPattern" and el.getParent() = this | + result = el.getTextValue() + ) + } + + predicate relocates(string fromPackage, string toPackage) { + this.getPattern() = fromPackage and this.getShadedPattern() = toPackage + } +} + +bindingset[package] +string getAShadedPackage(string package) { + result = package + or + exists(string originalPackage, string shadedPackage, MavenShadeRelocation relocDirective | + relocDirective.relocates(originalPackage, shadedPackage) + | + result = package.regexpReplaceAll("^" + originalPackage, shadedPackage) + ) +} From c10b15f9c38f08be0b6b506b10e9984723447777 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 7 Jun 2021 11:59:57 +0100 Subject: [PATCH 2/6] Recognise Shade plugin specified without an explicit groupId --- java/ql/src/semmle/code/xml/MavenPom.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/java/ql/src/semmle/code/xml/MavenPom.qll b/java/ql/src/semmle/code/xml/MavenPom.qll index 56c38dfd6ec9..5e614ad3a96e 100644 --- a/java/ql/src/semmle/code/xml/MavenPom.qll +++ b/java/ql/src/semmle/code/xml/MavenPom.qll @@ -492,7 +492,10 @@ class MavenPlugin extends ProtoPom { class MavenShadePlugin extends MavenPlugin { MavenShadePlugin() { - this.getGroup().getValue() = "org.apache.maven.plugins" and + ( + this.getGroup().getValue() = "org.apache.maven.plugins" or + not exists(this.getGroup()) // org.apache.maven.plugins is the default + ) and this.getArtifact().getValue() = "maven-shade-plugin" } From 52fe6f7b8aa0028e1053f8cfc5da3a7df674a6f3 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 7 Jun 2021 12:06:04 +0100 Subject: [PATCH 3/6] Document Maven shade support classes and predicates. --- java/ql/src/semmle/code/xml/MavenPom.qll | 28 ++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/java/ql/src/semmle/code/xml/MavenPom.qll b/java/ql/src/semmle/code/xml/MavenPom.qll index 5e614ad3a96e..b79bbe39ce1a 100644 --- a/java/ql/src/semmle/code/xml/MavenPom.qll +++ b/java/ql/src/semmle/code/xml/MavenPom.qll @@ -486,10 +486,16 @@ class MavenRepoJar extends File { } } +/** + * A `` element in a Maven POM file. + */ class MavenPlugin extends ProtoPom { MavenPlugin() { this.hasName("plugin") } } +/** + * A `maven-shade-plugin` plugin element in a Maven POM file. + */ class MavenShadePlugin extends MavenPlugin { MavenShadePlugin() { ( @@ -502,6 +508,9 @@ class MavenShadePlugin extends MavenPlugin { MavenShadeRelocation getARelocation() { result.getPlugin() = this } } +/** + * A `` specification within a `maven-shade-plugin` configuration. + */ class MavenShadeRelocation extends XMLElement { MavenShadePlugin plugin; @@ -510,25 +519,44 @@ class MavenShadeRelocation extends XMLElement { this.hasName("relocation") } + /** + * Gets the `` ancestor of this relocation. + */ MavenShadePlugin getPlugin() { result = plugin } + /** + * Gets the pattern this relocation replaces (e.g. `org.foo`) + */ string getPattern() { exists(XMLElement el | el.getName() = "pattern" and el.getParent() = this | result = el.getTextValue() ) } + /** + * Gets the shaded package that replaces `getPattern()` (e.g. `org.bar.shaded.org.foo`). + */ string getShadedPattern() { exists(XMLElement el | el.getName() = "shadedPattern" and el.getParent() = this | result = el.getTextValue() ) } + /** + * Holds if this `` specification relocates `fromPackage` to `toPackage`. + */ predicate relocates(string fromPackage, string toPackage) { this.getPattern() = fromPackage and this.getShadedPattern() = toPackage } } +/** + * Returns a package name that may be used to refer to `package` after shading, including `package` + * itself. + * + * For example, if `package` is `org.foo.sub` and we have a `` specification relocating + * `org.foo` to `org.bar.shaded.org.foo` then this will return `{"org.foo.sub", "org.bar.shaded.org.foo.sub"}`. + */ bindingset[package] string getAShadedPackage(string package) { result = package From 6b6bc1f6446681aa4a9a10c1fd4e50fed0950a39 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 10 Jun 2021 12:32:09 +0100 Subject: [PATCH 4/6] Account for shade reloc specifications within an expectation, and add a test case. --- java/ql/src/semmle/code/xml/MavenPom.qll | 19 +++---- .../ShadeRelocations.expected | 3 ++ .../shade-relocations/ShadeRelocations.ql | 6 +++ .../maven/shade-relocations/pom.xml | 49 +++++++++++++++++++ .../src/main/java/somepackage/Test.java | 3 ++ 5 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.expected create mode 100644 java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.ql create mode 100644 java/ql/test/library-tests/maven/shade-relocations/pom.xml create mode 100644 java/ql/test/library-tests/maven/shade-relocations/src/main/java/somepackage/Test.java diff --git a/java/ql/src/semmle/code/xml/MavenPom.qll b/java/ql/src/semmle/code/xml/MavenPom.qll index b79bbe39ce1a..14a6a2ae0adb 100644 --- a/java/ql/src/semmle/code/xml/MavenPom.qll +++ b/java/ql/src/semmle/code/xml/MavenPom.qll @@ -515,8 +515,11 @@ class MavenShadeRelocation extends XMLElement { MavenShadePlugin plugin; MavenShadeRelocation() { - this.getParent().(XMLElement).getParent().(XMLElement).getParent() = plugin and - this.hasName("relocation") + this = + [plugin, plugin.getAChild("executions").getAChild("execution")] + .getAChild("configuration") + .getAChild("relocations") + .getAChild("relocation") } /** @@ -527,20 +530,12 @@ class MavenShadeRelocation extends XMLElement { /** * Gets the pattern this relocation replaces (e.g. `org.foo`) */ - string getPattern() { - exists(XMLElement el | el.getName() = "pattern" and el.getParent() = this | - result = el.getTextValue() - ) - } + string getPattern() { result = this.getAChild("pattern").getTextValue() } /** * Gets the shaded package that replaces `getPattern()` (e.g. `org.bar.shaded.org.foo`). */ - string getShadedPattern() { - exists(XMLElement el | el.getName() = "shadedPattern" and el.getParent() = this | - result = el.getTextValue() - ) - } + string getShadedPattern() { result = this.getAChild("shadedPattern").getTextValue() } /** * Holds if this `` specification relocates `fromPackage` to `toPackage`. diff --git a/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.expected b/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.expected new file mode 100644 index 000000000000..78772ac6aac3 --- /dev/null +++ b/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.expected @@ -0,0 +1,3 @@ +| org.codehaus.plexus.abc | org.shaded.plexus.abc | +| org.codehaus.plexus.def | org.shaded.plexus.def | +| org.codehaus.plexus.util | org.shaded.plexus.util | diff --git a/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.ql b/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.ql new file mode 100644 index 000000000000..7be992ee2fc7 --- /dev/null +++ b/java/ql/test/library-tests/maven/shade-relocations/ShadeRelocations.ql @@ -0,0 +1,6 @@ +import java +import semmle.code.xml.MavenPom + +from string pkgFrom, string pkgTo, MavenShadeRelocation reloc +where reloc.relocates(pkgFrom, pkgTo) +select pkgFrom, pkgTo diff --git a/java/ql/test/library-tests/maven/shade-relocations/pom.xml b/java/ql/test/library-tests/maven/shade-relocations/pom.xml new file mode 100644 index 000000000000..7a32e6c5f4d5 --- /dev/null +++ b/java/ql/test/library-tests/maven/shade-relocations/pom.xml @@ -0,0 +1,49 @@ + + 4.0.0 + + MYGROUP + MYARTIFACT + 0.1-SNAPSHOT + + + + + org.apache.maven.plugins + maven-shade-plugin + 3.2.4 + + + + org.codehaus.plexus.abc + org.shaded.plexus.abc + + + org.codehaus.plexus.def + org.shaded.plexus.def + + + + + + package + + shade + + + + + org.codehaus.plexus.util + org.shaded.plexus.util + + org.codehaus.plexus.util.xml.Xpp3Dom + org.codehaus.plexus.util.xml.pull.* + + + + + + + + + + diff --git a/java/ql/test/library-tests/maven/shade-relocations/src/main/java/somepackage/Test.java b/java/ql/test/library-tests/maven/shade-relocations/src/main/java/somepackage/Test.java new file mode 100644 index 000000000000..7b51bd46d99c --- /dev/null +++ b/java/ql/test/library-tests/maven/shade-relocations/src/main/java/somepackage/Test.java @@ -0,0 +1,3 @@ +package somepackage; + +public class Test { } // Nothing to see here; the real test is in pom.xml; this is just to avoid a warning that the extractor didn't find any Java code. From 7229db2ce4ecd4117032d396f61c0b214337f165 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 10 Jun 2021 12:35:24 +0100 Subject: [PATCH 5/6] Fix package of SnakeYaml's Yaml class --- java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll b/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll index a029ce5d1611..f290b5da70fe 100644 --- a/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll +++ b/java/ql/src/semmle/code/java/frameworks/SnakeYaml.qll @@ -28,10 +28,7 @@ class SafeSnakeYamlConstruction extends ClassInstanceExpr { * The class `org.yaml.snakeyaml.Yaml`. */ class Yaml extends RefType { - Yaml() { - this.getASupertype*() - .hasQualifiedName(getAShadedPackage("org.yaml.snakeyaml.constructor"), "Yaml") - } + Yaml() { this.getASupertype*().hasQualifiedName(getAShadedPackage("org.yaml.snakeyaml"), "Yaml") } } private class SafeYamlConstructionFlowConfig extends DataFlow2::Configuration { From d6663f8164a8787c516ecfc888901716417a7cb3 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Thu, 10 Jun 2021 12:36:51 +0100 Subject: [PATCH 6/6] Improve doc style Co-authored-by: Marcono1234 --- java/ql/src/semmle/code/xml/MavenPom.qll | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/semmle/code/xml/MavenPom.qll b/java/ql/src/semmle/code/xml/MavenPom.qll index 14a6a2ae0adb..bed1fbf1a2a8 100644 --- a/java/ql/src/semmle/code/xml/MavenPom.qll +++ b/java/ql/src/semmle/code/xml/MavenPom.qll @@ -546,11 +546,11 @@ class MavenShadeRelocation extends XMLElement { } /** - * Returns a package name that may be used to refer to `package` after shading, including `package` + * Gets a package name that may be used to refer to `package` after shading, including `package` * itself. * * For example, if `package` is `org.foo.sub` and we have a `` specification relocating - * `org.foo` to `org.bar.shaded.org.foo` then this will return `{"org.foo.sub", "org.bar.shaded.org.foo.sub"}`. + * `org.foo` to `org.bar.shaded.org.foo` then the results will be `{"org.foo.sub", "org.bar.shaded.org.foo.sub"}`. */ bindingset[package] string getAShadedPackage(string package) {