Reject image tags that are neither mapped nor valid image hashes before building the manifest path

external/storm-hdfs-oci/src/main/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPlugin.java

@@ -23,6 +23,7 @@
 import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.io.InputStreamReader;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
@@ -84,6 +85,16 @@ public class LocalOrHdfsImageTagToManifestPlugin implements OciImageTagToManifes
 
     private static final String ALPHA_NUMERIC = "[a-zA-Z0-9]+";
 
+    /**
+     * Check that a string can be used as an image hash, i.e. it consists of exactly
+     * {@link #SHA256_HASH_LENGTH} alphanumeric characters.
+     * @param hash the string to check
+     * @return true if the string has the shape of an image hash
+     */
+    private static boolean isValidHash(String hash) {
+        return hash != null && hash.length() == SHA256_HASH_LENGTH && hash.matches(ALPHA_NUMERIC);
+    }
+
     @Override
     public void init(Map<String, Object> conf) throws IOException {
         this.conf = conf;
@@ -213,7 +224,7 @@ private static Map<String, String> readImageToHashFile(BufferedReader br, String
             String[] imageTagArray = imageTags.split(",");
             String hash = line.substring(index + 1);
 
-            if (!hash.matches(ALPHA_NUMERIC) || hash.length() != SHA256_HASH_LENGTH) {
+            if (!isValidHash(hash)) {
                 LOG.warn("Malformed image hash: " + hash);
                 continue;
             }
@@ -229,6 +240,10 @@ private static Map<String, String> readImageToHashFile(BufferedReader br, String
     @Override
     public synchronized ImageManifest getManifestFromImageTag(String imageTag) throws IOException {
         String hash = getHashFromImageTag(imageTag);
+        if (!isValidHash(hash)) {
+            throw new IOException("Cannot get manifest for image tag " + imageTag
+                + ": " + hash + " is not a valid image hash");
+        }
         ImageManifest manifest = manifestCache.get(hash);
         if (manifest != null) {
             return manifest;
@@ -271,12 +286,16 @@ public synchronized String getHashFromImageTag(String imageTag) {
 
         // 1) Go to local file
         // 2) Go to HDFS
-        // 3) Use tag as is/Assume tag is the hash
+        // 3) Use tag as is/Assume tag is the hash; only acceptable if the tag looks like a hash
         if ((hash = localImageToHashCache.get(imageTag)) != null) {
             return hash;
         } else if ((hash = hdfsImageToHashCache.get(imageTag)) != null) {
             return hash;
         } else {
+            if (!isValidHash(imageTag)) {
+                throw new UncheckedIOException(new IOException("Image tag " + imageTag
+                    + " is not in the image-tag-to-hash files and is not a valid image hash itself"));
+            }
             return imageTag;
         }
     }

external/storm-hdfs-oci/src/test/java/org/apache/storm/container/oci/LocalOrHdfsImageTagToManifestPluginTest.java

@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.storm.container.oci;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import org.apache.storm.DaemonConfig;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+public class LocalOrHdfsImageTagToManifestPluginTest {
+
+    private static final String KNOWN_HASH = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef";
+    private static final String UNKNOWN_HASH = "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210";
+    //same length as a hash, but made of characters that would still escape the manifest directory
+    private static final String HASH_LENGTH_PATH = "../../../user/foo/bar/" + "a".repeat(42);
+
+    @TempDir
+    Path tempDir;
+
+    private LocalOrHdfsImageTagToManifestPlugin createPlugin() throws IOException {
+        Path hashFile = tempDir.resolve("image-tag-to-hash");
+        Files.write(hashFile, ("busybox:latest:" + KNOWN_HASH + "\n").getBytes(StandardCharsets.UTF_8));
+
+        Map<String, Object> conf = new HashMap<>();
+        conf.put("storm.oci.local.or.hdfs.image.tag.to.manifest.plugin.local.hash.file", hashFile.toString());
+        conf.put(DaemonConfig.STORM_OCI_IMAGE_HDFS_TOPLEVEL_DIR, "/storm/oci");
+
+        LocalOrHdfsImageTagToManifestPlugin plugin = new LocalOrHdfsImageTagToManifestPlugin();
+        plugin.init(conf);
+        return plugin;
+    }
+
+    @Test
+    public void testKnownImageTagIsMappedToItsHash() throws Exception {
+        assertEquals(KNOWN_HASH, createPlugin().getHashFromImageTag("busybox:latest"));
+    }
+
+    @Test
+    public void testUnmappedImageTagIsUsedAsHashWhenItLooksLikeOne() throws Exception {
+        assertEquals(UNKNOWN_HASH, createPlugin().getHashFromImageTag(UNKNOWN_HASH));
+    }
+
+    @Test
+    public void testUnmappedImageTagThatIsNotAHashIsRejected() throws Exception {
+        LocalOrHdfsImageTagToManifestPlugin plugin = createPlugin();
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("../../../user/foo/bar"));
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("busybox:unknown"));
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag(".."));
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag("/etc/passwd"));
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag(HASH_LENGTH_PATH));
+        assertThrows(UncheckedIOException.class, () -> plugin.getHashFromImageTag(UNKNOWN_HASH + "a"));
+    }
+
+    @Test
+    public void testGetManifestFromImageTagRejectsUnmappedNonHashTag() throws Exception {
+        LocalOrHdfsImageTagToManifestPlugin plugin = createPlugin();
+        assertThrows(UncheckedIOException.class, () -> plugin.getManifestFromImageTag("../../../user/foo/bar"));
+        assertThrows(UncheckedIOException.class, () -> plugin.getManifestFromImageTag(HASH_LENGTH_PATH));
+    }
+}