CVE-2022-24816: Improper Control of Generation of Code ('Code Injection') (CWE-94)
value: JAI-EXT is an open-source project which aims to extend the Java Advanced Imaging (JAI) API. Programs allowing Jiffle script to be provided via network request can lead to a Remote Code Execution as the Jiffle script is compiled into Java code via Janino, and executed. In particular, this affects the downstream GeoServer project. Version 1.2.22 will contain a patch that disables the ability to inject malicious code into the resulting script. Users unable to upgrade may negate the ability to compile Jiffle scripts from the final application, by removing janino-x.y.z.jar from the classpath.
CWE description: The product constructs all or part of a code segment using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the syntax or behavior of the intended code segment.
CVSS v3 base score: 9.8
Published: 2022-04-13T21:15Z
Repository: https://github.com/geosolutions-it/jai-ext
Language: Java
Files changed: jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/Jiffle.java, jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/parser/node/Script.java, jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/JiffleRuntimeTest.java, jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/ScriptTest.java
Commit date: 2022-04-11 10:28:01 +0200
Commit message: Validate Jiffle input variable names according to grammar, escape javadocs when including Jiffle sources in output

Diff: diff --git a/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/Jiffle.java b/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/Jiffle.java
index c3dc9ce..7252689 100644
--- a/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/Jiffle.java
+++ b/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/Jiffle.java
@@ -126,11 +126,11 @@ import it.geosolutions.jaiext.jiffle.runtime.JiffleRuntime;
  */
 public class Jiffle {
     
     public static final Logger LOGGER = Logger.getLogger(Jiffle.class.getName());
     
-    private static Pattern BLOCK_COMMENT_STRIPPER = Pattern.compile("(?:/\\*(?:[^*]|(?:\\*+[^*/]))*\\*+/)");
+    boolean includeScript;
 
     /** 
      * Constants for runtime model. Jiffle supports two runtime models:
      * <ol type="1">
      * <li>
@@ -270,11 +270,24 @@ public class Jiffle {
         init();
         setScript(scriptFile);
         setImageParams(params);
         compile();
     }
-    
+
+    /**
+     * Returns true if the script is to be included in the output
+     *
+     * @return
+     */
+    public boolean isIncludeScript() {
+        return includeScript;
+    }
+
+    public void setIncludeScript(boolean includeScript) {
+        this.includeScript = includeScript;
+    }
+
     /**
      * Sets the script. Calling this method will clear any previous script
      * and runtime objects.
      * 
      * @param script a Jiffle script
@@ -479,11 +492,11 @@ public class Jiffle {
      * @throws JiffleException  if the script has not been compiled or if errors
      *         occur in creating the runtime instance
      */
     public JiffleRuntime getRuntimeInstance(Jiffle.RuntimeModel model) throws
             it.geosolutions.jaiext.jiffle.JiffleException {
-        return createRuntimeInstance(model, getRuntimeBaseClass(model), false);
+        return createRuntimeInstance(model, getRuntimeBaseClass(model), includeScript);
     }
 
     /**
      * Gets the runtime object for this script. 
      * <p>
@@ -510,11 +523,11 @@ public class Jiffle {
         if (model == null) {
             throw new it.geosolutions.jaiext.jiffle.JiffleException(baseClass.getName() +
                     " does not implement a required Jiffle runtime interface");
         }
 
-        return (T) createRuntimeInstance(model, baseClass, false);
+        return (T) createRuntimeInstance(model, baseClass, includeScript);
     }
 
     private JiffleRuntime createRuntimeInstance(RuntimeModel model, Class<? extends JiffleRuntime> runtimeClass, boolean scriptInDocs) throws
             
             it.geosolutions.jaiext.jiffle.JiffleException {
@@ -556,11 +569,18 @@ public class Jiffle {
                 ((JiffleIndirectRuntime) runtime).setDestinationBands(destinationBands);
             }
             return runtime;
 
         } catch (Exception ex) {
-            throw new it.geosolutions.jaiext.jiffle.JiffleException("Runtime source error for source: " + runtimeSource, ex);
+            // do not display the source code in indirect runtime exception messages
+            if (model == RuntimeModel.INDIRECT) {
+                if (LOGGER.isLoggable(Level.FINE)) {
+                    LOGGER.fine("Runtime source error for source: " + runtimeSource);
+                }
+                throw new JiffleException("Runtime source error", ex);
+            }
+            throw new JiffleException("Runtime source error for source: " + runtimeSource, ex);
         }
     }
 
     /**
      * Gets a copy of the Java source for the runtime class. The 
@@ -611,26 +631,21 @@ public class Jiffle {
         }
         
         return baseClass;
     }
 
-    private String createRuntimeSource(RuntimeModel model, String baseClassName, boolean scriptInDocs) {
+    private String createRuntimeSource(RuntimeModel model, String baseClassName,
+                                       boolean scriptInDocs) {
+        SourceWriter writer = new SourceWriter(model);
         if (scriptInDocs) {
-            throw new RuntimeException("Do no know how to clean the block comments yet");
+            writer.setScript(theScript);
         }
-
-        SourceWriter writer = new SourceWriter(model);
-        writer.setScript(stripComments(theScript));
         writer.setBaseClassName(baseClassName);
         scriptModel.write(writer);
         return writer.getSource();
     }
 
-    private String stripComments(String theScript) {
-        return BLOCK_COMMENT_STRIPPER.matcher(theScript).replaceAll("");
-    }
-
     /**
      * Initializes this object's name and runtime base class.
      */
     private void init() {
         Jiffle.refCount++ ;
@@ -646,10 +661,11 @@ public class Jiffle {
         // TODO if necessary
     }
     
     /**
      * Builds the parse tree from the script.
+     *
      * @param script
      */
     private static Jiffle.Result<ParseTree> parseScript(String script) {
         CharStream input = CharStreams.fromString(script);
         
diff --git a/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/parser/node/Script.java b/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/parser/node/Script.java
index 9de8193..c7c550f 100644
--- a/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/parser/node/Script.java
+++ b/jt-jiffle/jt-jiffle-language/src/main/java/it/geosolutions/jaiext/jiffle/parser/node/Script.java
@@ -50,17 +50,21 @@ import it.geosolutions.jaiext.jiffle.parser.JiffleParserException;
 import it.geosolutions.jaiext.jiffle.parser.OptionLookup;
 import it.geosolutions.jaiext.jiffle.parser.RepeatedReadOptimizer;
 import it.geosolutions.jaiext.jiffle.parser.UndefinedOptionException;
 
 import java.util.Collections;
-import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.regex.Pattern;
 
 /** @author michael */
 public class Script implements Node {
+
+    /** <pre>ID      : (Letter) (Letter | UNDERSCORE | Digit | Dot)*</pre> */
+    private static final Pattern VALID_IDENTIFIER = Pattern.compile("^[a-zA-Z][_a-zA-Z0-9\\.]*$");
+
     private final StatementList stmts;
     private final RepeatedReadOptimizer readOptimizer;
     private Map<String, String> options;
     private Set<String> sourceImages;
     private Set<String> destImages;
@@ -77,10 +81,24 @@ public class Script implements Node {
         this.sourceImages = sourceImages;
         this.destImages = destImages;
         this.globals = globals;
         this.stmts = stmts;
         this.readOptimizer = readOptimizer;
+        validate();
+    }
+
+    private void validate() {
+        for (String name : sourceImages) {
+            if (!VALID_IDENTIFIER.matcher(name).matches()) {
+                throw new JiffleParserException("Invalid source image name: " + name);
+            }
+        }
+        for (String name : destImages) {
+            if (!VALID_IDENTIFIER.matcher(name).matches()) {
+                throw new JiffleParserException("Invalid dest image name: " + name);
+            }
+        }
     }
 
     public void write(SourceWriter w) {
         // class header
         String packageName = "it.geosolutions.jaiext.jiffle.runtime";
@@ -95,15 +113,17 @@ public class Script implements Node {
         String script = w.getScript();
         if (script != null) {
             String[] lines = script.split("\n");
             w.line("/**");
             w.line(" * Java runtime class generated from the following Jiffle script: ");
-            w.line(" *<code>");
+            w.line(" *<pre>");
             for (String line : lines) {
-                w.append(" * ").append(line).newLine();
+                // In case the script itself includes comments, they best to be escaped
+                String escaped = line.replace("*/", "*&#47;").replace("/*", "&#47;*");
+                w.append(" * ").append(escaped).newLine();
             }
-            w.line(" *</code>");
+            w.line(" *</pre>");
             w.line(" */");
         }
 
         // class declaration
         String template = "public class %s extends %s {";
diff --git a/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/JiffleRuntimeTest.java b/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/JiffleRuntimeTest.java
new file mode 100644
index 0000000..f1b50be
--- /dev/null
+++ b/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/JiffleRuntimeTest.java
@@ -0,0 +1,61 @@
+/* JAI-Ext - OpenSource Java Advanced Image Extensions Library
+ *    http://www.geo-solutions.it/
+ *    Copyright 2018 GeoSolutions
+ *
+ * Licensed 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 it.geosolutions.jaiext.jiffle.runtime;
+
+import it.geosolutions.jaiext.jiffle.Jiffle;
+import it.geosolutions.jaiext.jiffle.JiffleException;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class JiffleRuntimeTest {
+
+    @Test
+    public void testSkipSource() throws Exception {
+        Jiffle jiffle = getJiffleWithComment();
+        String source = jiffle.getRuntimeSource(false);
+
+        // by default, it does not contain the source
+        assertFalse(source.contains("This is a comment"));
+    }
+
+    @Test
+    public void testEscapeSource() throws Exception {
+        Jiffle jiffle = getJiffleWithComment();
+        String source = jiffle.getRuntimeSource(true);
+
+        // source is in the javadocs, and properly escaped
+        assertTrue(source.contains("* &#47;* This is a comment *&#47;"));
+        assertTrue(source.contains("* dest=10;"));
+    }
+
+    private Jiffle getJiffleWithComment() throws JiffleException {
+        String script = "/* This is a comment */\ndest=10;";
+        Jiffle jiffle = new Jiffle();
+        jiffle.setScript(script);
+        Map<String, Jiffle.ImageRole> params = new HashMap<>();
+        params.put("dest", Jiffle.ImageRole.DEST);
+        jiffle.setImageParams(params);
+
+        jiffle.compile();
+        return jiffle;
+    }
+}
diff --git a/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/ScriptTest.java b/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/ScriptTest.java
new file mode 100644
index 0000000..21e857b
--- /dev/null
+++ b/jt-jiffle/jt-jiffle-language/src/test/java/it/geosolutions/jaiext/jiffle/runtime/ScriptTest.java
@@ -0,0 +1,73 @@
+/* JAI-Ext - OpenSource Java Advanced Image Extensions Library
+ *    http://www.geo-solutions.it/
+ *    Copyright 2018 GeoSolutions
+ *
+ * Licensed 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 it.geosolutions.jaiext.jiffle.runtime;
+
+import it.geosolutions.jaiext.jiffle.Jiffle;
+import it.geosolutions.jaiext.jiffle.JiffleException;
+import it.geosolutions.jaiext.jiffle.parser.JiffleParserException;
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+public class ScriptTest {
+
+    @Test
+    public void testValidateSourceImages() throws Exception {
+        checkInvalidSourceName("a b");
+        checkInvalidSourceName("99a");
+        checkInvalidSourceName("#abc");
+        checkInvalidSourceName("/abc");
+        checkInvalidSourceName("{abc");
+    }
+
+    @Test
+    public void testValidateDestinationImages() throws Exception {
+        checkInvalidDestinationName("a b");
+        checkInvalidDestinationName("99a");
+        checkInvalidDestinationName("#abc");
+        checkInvalidDestinationName("/abc");
+        checkInvalidDestinationName("{abc");
+    }
+
+    private void checkInvalidSourceName(String name) throws JiffleException {
+        Jiffle jiffle = new Jiffle();
+        jiffle.setScript("dst=0;");
+        Map<String, Jiffle.ImageRole> params = new HashMap<>();
+        params.put(name, Jiffle.ImageRole.SOURCE);
+        jiffle.setImageParams(params);
+
+        JiffleParserException exception =
+                Assert.assertThrows(JiffleParserException.class, () -> jiffle.compile());
+        assertEquals("Invalid source image name: " + name, exception.getMessage());
+    }
+
+    private void checkInvalidDestinationName(String name) throws JiffleException {
+        Jiffle jiffle = new Jiffle();
+        jiffle.setScript("dst=0;");
+        Map<String, Jiffle.ImageRole> params = new HashMap<>();
+        params.put(name, Jiffle.ImageRole.DEST);
+        jiffle.setImageParams(params);
+
+        JiffleParserException exception =
+                Assert.assertThrows(JiffleParserException.class, () -> jiffle.compile());
+        assertEquals("Invalid dest image name: " + name, exception.getMessage());
+    }
+}

Vulnerable code:     private static Pattern BLOCK_COMMENT_STRIPPER = Pattern.compile("(?:/\\*(?:[^*]|(?:\\*+[^*/]))*\\*+/)");
    
        return createRuntimeInstance(model, getRuntimeBaseClass(model), false);
        return (T) createRuntimeInstance(model, baseClass, false);
            throw new it.geosolutions.jaiext.jiffle.JiffleException("Runtime source error for source: " + runtimeSource, ex);
    private String createRuntimeSource(RuntimeModel model, String baseClassName, boolean scriptInDocs) {
            throw new RuntimeException("Do no know how to clean the block comments yet");

        SourceWriter writer = new SourceWriter(model);
        writer.setScript(stripComments(theScript));
    private String stripComments(String theScript) {
        return BLOCK_COMMENT_STRIPPER.matcher(theScript).replaceAll("");
    }

import java.util.List;
            w.line(" *<code>");
                w.append(" * ").append(line).newLine();
            w.line(" *</code>");
Fixed code:     boolean includeScript;

    /**
     * Returns true if the script is to be included in the output
     *
     * @return
     */
    public boolean isIncludeScript() {
        return includeScript;
    }

    public void setIncludeScript(boolean includeScript) {
        this.includeScript = includeScript;
    }

        return createRuntimeInstance(model, getRuntimeBaseClass(model), includeScript);
        return (T) createRuntimeInstance(model, baseClass, includeScript);
            // do not display the source code in indirect runtime exception messages
            if (model == RuntimeModel.INDIRECT) {
                if (LOGGER.isLoggable(Level.FINE)) {
                    LOGGER.fine("Runtime source error for source: " + runtimeSource);
                }
                throw new JiffleException("Runtime source error", ex);
            }
            throw new JiffleException("Runtime source error for source: " + runtimeSource, ex);
    private String createRuntimeSource(RuntimeModel model, String baseClassName,
                                       boolean scriptInDocs) {
        SourceWriter writer = new SourceWriter(model);
            writer.setScript(theScript);
     *
import java.util.regex.Pattern;

    /** <pre>ID      : (Letter) (Letter | UNDERSCORE | Digit | Dot)*</pre> */
    private static final Pattern VALID_IDENTIFIER = Pattern.compile("^[a-zA-Z][_a-zA-Z0-9\\.]*$");

        validate();
    }

    private void validate() {
        for (String name : sourceImages) {
            if (!VALID_IDENTIFIER.matcher(name).matches()) {
                throw new JiffleParserException("Invalid source image name: " + name);
            }
        }
        for (String name : destImages) {
            if (!VALID_IDENTIFIER.matcher(name).matches()) {
                throw new JiffleParserException("Invalid dest image name: " + name);
            }
        }
            w.line(" *<pre>");
                // In case the script itself includes comments, they best to be escaped
                String escaped = line.replace("*/", "*&#47;").replace("/*", "&#47;*");
                w.append(" * ").append(escaped).newLine();
            w.line(" *</pre>");
/* JAI-Ext - OpenSource Java Advanced Image Extensions Library
 *    http://www.geo-solutions.it/
 *    Copyright 2018 GeoSolutions
 *
 * Licensed 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 it.geosolutions.jaiext.jiffle.runtime;

import it.geosolutions.jaiext.jiffle.Jiffle;
import it.geosolutions.jaiext.jiffle.JiffleException;
import org.junit.Test;

import java.util.HashMap;
import java.util.Map;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;

public class JiffleRuntimeTest {

    @Test
    public void testSkipSource() throws Exception {
        Jiffle jiffle = getJiffleWithComment();
        String source = jiffle.getRuntimeSource(false);

        // by default, it does not contain the source
        assertFalse(source.contains("This is a comment"));
    }

    @Test
    public void testEscapeSource() throws Exception {
        Jiffle jiffle = getJiffleWithComment();
        String source = jiffle.getRuntimeSource(true);

        // source is in the javadocs, and properly escaped
        assertTrue(source.contains("* &#47;* This is a comment *&#47;"));
        assertTrue(source.contains("* dest=10;"));
    }

    private Jiffle getJiffleWithComment() throws JiffleException {
        String script = "/* This is a comment */\ndest=10;";
        Jiffle jiffle = new Jiffle();
        jiffle.setScript(script);
        Map<String, Jiffle.ImageRole> params = new HashMap<>();
        params.put("dest", Jiffle.ImageRole.DEST);
        jiffle.setImageParams(params);

        jiffle.compile();
        return jiffle;
    }
}
/* JAI-Ext - OpenSource Java Advanced Image Extensions Library
 *    http://www.geo-solutions.it/
 *    Copyright 2018 GeoSolutions
 *
 * Licensed 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 it.geosolutions.jaiext.jiffle.runtime;

import it.geosolutions.jaiext.jiffle.Jiffle;
import it.geosolutions.jaiext.jiffle.JiffleException;
import it.geosolutions.jaiext.jiffle.parser.JiffleParserException;
import org.junit.Assert;
import org.junit.Test;

import java.util.HashMap;
import java.util.Map;

import static org.junit.Assert.assertEquals;

public class ScriptTest {

    @Test
    public void testValidateSourceImages() throws Exception {
        checkInvalidSourceName("a b");
        checkInvalidSourceName("99a");
        checkInvalidSourceName("#abc");
        checkInvalidSourceName("/abc");
        checkInvalidSourceName("{abc");
    }

    @Test
    public void testValidateDestinationImages() throws Exception {
        checkInvalidDestinationName("a b");
        checkInvalidDestinationName("99a");
        checkInvalidDestinationName("#abc");
        checkInvalidDestinationName("/abc");
        checkInvalidDestinationName("{abc");
    }

    private void checkInvalidSourceName(String name) throws JiffleException {
        Jiffle jiffle = new Jiffle();
        jiffle.setScript("dst=0;");
        Map<String, Jiffle.ImageRole> params = new HashMap<>();
        params.put(name, Jiffle.ImageRole.SOURCE);
        jiffle.setImageParams(params);

        JiffleParserException exception =
                Assert.assertThrows(JiffleParserException.class, () -> jiffle.compile());
        assertEquals("Invalid source image name: " + name, exception.getMessage());
    }

    private void checkInvalidDestinationName(String name) throws JiffleException {
        Jiffle jiffle = new Jiffle();
        jiffle.setScript("dst=0;");
        Map<String, Jiffle.ImageRole> params = new HashMap<>();
        params.put(name, Jiffle.ImageRole.DEST);
        jiffle.setImageParams(params);

        JiffleParserException exception =
                Assert.assertThrows(JiffleParserException.class, () -> jiffle.compile());
        assertEquals("Invalid dest image name: " + name, exception.getMessage());
    }
}