[java-identity-provider] branch main updated: IDP-1668 Add some basic defense against badly formed paths
Rod Widdowson
rdw at steadingsoftware.com
Tue Sep 8 15:19:41 UTC 2020
This is an automated email from the git hooks/post-receive script.
rdw pushed a commit to branch main
in repository java-identity-provider.
View the commit online:
http://git.shibboleth.net/view/?p=java-identity-provider.git;a=commit;h=a303ac880cdb9437014607186a8b82a685356597
The following commit(s) were added to refs/heads/main by this push:
new a303ac880 IDP-1668 Add some basic defense against badly formed paths
a303ac880 is described below
commit a303ac880cdb9437014607186a8b82a685356597
Author: Rod Widdowson <rdw at steadingsoftware.com>
AuthorDate: Tue Sep 8 16:17:04 2020 +0100
IDP-1668 Add some basic defense against badly formed paths
https://issues.shibboleth.net/jira/browse/IDP-1668
The concern is that the plugin could inject a path which is outside
where we should be operating ("../../etc/passwd") or some such.
To that end we always canonicalize the source (distribution dir) and
target (idpHome) and then we also check (via canonicalized paths)
that every from file comes from the distribution and every to file
is inside idpHome.
---
.../idp/installer/plugin/impl/PluginInstaller.java | 63 +++++++++++++++++++++-
1 file changed, 62 insertions(+), 1 deletion(-)
diff --git a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
index 03321f312..198ce4a09 100644
--- a/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
+++ b/idp-installer/src/main/java/net/shibboleth/idp/installer/plugin/impl/PluginInstaller.java
@@ -70,6 +70,7 @@ import net.shibboleth.utilities.java.support.annotation.constraint.NotEmpty;
import net.shibboleth.utilities.java.support.collection.Pair;
import net.shibboleth.utilities.java.support.component.AbstractInitializableComponent;
import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
+import net.shibboleth.utilities.java.support.component.ComponentSupport;
import net.shibboleth.utilities.java.support.httpclient.HttpClientBuilder;
import net.shibboleth.utilities.java.support.logic.Constraint;
import net.shibboleth.utilities.java.support.primitive.StringSupport;
@@ -118,6 +119,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
* @param home Where we are working from
*/
public void setIdpHome(@Nonnull final Path home) {
+ ComponentSupport.ifInitializedThrowUnmodifiabledComponentException(this);
idpHome = Constraint.isNotNull(home, "IdPHome should be non-null");
}
@@ -149,6 +151,49 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
httpClient = Constraint.isNotNull(what, "HttpClient should be non-null");
}
+ /** Return the canonical path.
+ * @param from the path we get given
+ * @return the canonicalized one
+ * @throws IOException as from {@link File#getCanonicalFile()}
+ */
+ private static Path canonicalPath(final Path from) throws IOException {
+ return from.toFile().getCanonicalFile().toPath();
+ }
+
+ /** Check that the provide path is inside {@link #idpHome}.
+ * @param to the path to check.
+ * @throws BuildException if it isn't
+ */
+ private void policeTo(final Path to) throws BuildException {
+ try {
+ final Path canonicalTo = canonicalPath(to);
+ if (!canonicalTo.startsWith(idpHome)) {
+ LOG.error("File destination {} ({}) was illegal (not inside {}", to, canonicalTo, idpHome);
+ throw new BuildException("Illegal file destination");
+ }
+ } catch (final IOException e) {
+ LOG.error("Error checking destination {}", to, e);
+ throw new BuildException(e);
+ }
+ }
+
+ /** Check that the provide path is inside {@link #distribution}.
+ * @param from the path to check.
+ * @throws BuildException if it isn't
+ */
+ private void policeFrom(final Path from) throws BuildException {
+ try {
+ final Path canonicalFrom = canonicalPath(from);
+ if (!canonicalFrom.startsWith(distribution)) {
+ LOG.error("File source {} ({}) was illegal (not inside {}", from, canonicalFrom, distribution);
+ throw new BuildException("Illegal file source");
+ }
+ } catch (final IOException e) {
+ LOG.error("Error checking destination {}", from, e);
+ throw new BuildException(e);
+ }
+ }
+
/** Install the plugin from the provided URL. Involves downloading
* the file and then doing a {@link #installPlugin(Path, String)}.
* @param baseURL where we get the files from
@@ -218,6 +263,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
try {
for (final Pair<URL, Path> pair : description.getExternalFilePathsToCopy()) {
final Path to = idpHome.resolve(pair.getSecond());
+ policeTo(to);
if (Files.exists(to)) {
LOG.warn("{} exists, not copied", to);
continue;
@@ -289,7 +335,9 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
}
final Path from = distribution.resolve(p);
+ policeFrom(from);
final Path to = idpHome.resolve(p);
+ policeTo(to);
if (Files.exists(to)) {
LOG.debug("File {} exists, skipping", to);
continue;
@@ -433,7 +481,7 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
LOG.error("No contents unpacked from {}", fullName);
throw new BuildException("Distro was empty");
}
- distribution = contents.next();
+ distribution = canonicalPath(contents.next());
if (contents.hasNext()) {
LOG.error("Too many packages in distributions {}", fullName);
throw new BuildException("Too many packages in distributions");
@@ -550,6 +598,19 @@ public final class PluginInstaller extends AbstractInitializableComponent implem
}
}
+ /** {@inheritDoc} */
+ protected void doInitialize() throws ComponentInitializationException {
+ if (idpHome == null) {
+ throw new ComponentInitializationException("Idp Home should be set");
+ }
+ try {
+ idpHome = canonicalPath(idpHome);
+ } catch (final IOException e) {
+ LOG.error("Could not canonicalize idp home", e);
+ throw new ComponentInitializationException(e);
+ }
+ }
+
/**
* Return a list of the installed plugins.
* @return All the plugins.
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list