From f3ef93fa8abc6118b3cc47f8e451ab0885ee9362 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 11 Jun 2021 16:56:35 +0200 Subject: [PATCH] Make sinks more specific, improve tests --- .../code/java/security/GroovyInjection.qll | 12 +- .../CWE-094/GroovyClassLoaderTest.java | 59 ++-- .../groovy/lang/GroovyClassLoader.java | 289 ++++++++++++++++-- .../org/codehaus/groovy/ast/ClassNode.java | 252 +++++++++++++++ 4 files changed, 570 insertions(+), 42 deletions(-) create mode 100644 java/ql/test/stubs/groovy-all-3.0.7/org/codehaus/groovy/ast/ClassNode.java diff --git a/java/ql/src/semmle/code/java/security/GroovyInjection.qll b/java/ql/src/semmle/code/java/security/GroovyInjection.qll index 09625ce8d10..475037db561 100644 --- a/java/ql/src/semmle/code/java/security/GroovyInjection.qll +++ b/java/ql/src/semmle/code/java/security/GroovyInjection.qll @@ -1,7 +1,4 @@ -/** - * Provides classes and predicates for Groovy Code Injection - * taint-tracking configuration. - */ +/** Provides classes to reason about Groovy code injection attacks. */ import java import semmle.code.java.dataflow.DataFlow @@ -58,7 +55,12 @@ private class DefaultLdapInjectionSinkModel extends SinkModelCsv { "groovy.util;Eval;false;x;(Object,String);;Argument[1];groovy", "groovy.util;Eval;false;xy;(Object,Object,String);;Argument[2];groovy", "groovy.util;Eval;false;xyz;(Object,Object,Object,String);;Argument[3];groovy", - "groovy.lang;GroovyClassLoader;false;parseClass;;;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource);;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource,boolean);;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(InputStream,String);;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(Reader,String);;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(String);;Argument[0];groovy", + "groovy.lang;GroovyClassLoader;false;parseClass;(String,String);;Argument[0];groovy", "org.codehaus.groovy.control;CompilationUnit;false;compile;;;Argument[-1];groovy" ] } diff --git a/java/ql/test/query-tests/security/CWE-094/GroovyClassLoaderTest.java b/java/ql/test/query-tests/security/CWE-094/GroovyClassLoaderTest.java index 86de1702faa..437a1df6f71 100644 --- a/java/ql/test/query-tests/security/CWE-094/GroovyClassLoaderTest.java +++ b/java/ql/test/query-tests/security/CWE-094/GroovyClassLoaderTest.java @@ -1,35 +1,54 @@ -import groovy.lang.GroovyClassLoader; -import groovy.lang.GroovyCodeSource; -import groovy.lang.GroovyObject; - +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.StringReader; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.IOException; +import groovy.lang.GroovyClassLoader; +import groovy.lang.GroovyCodeSource; public class GroovyClassLoaderTest extends HttpServlet { protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - // "groovy.lang;GroovyClassLoader;false;parseClass;;;Argument[0];groovy", - try { - String script = request.getParameter("script"); - final GroovyClassLoader classLoader = new GroovyClassLoader(); - Class groovy = classLoader.parseClass(script); // $hasGroovyInjection - GroovyObject groovyObj = (GroovyObject) groovy.newInstance(); - - } catch (Exception e) { - // Ignore - } - try { + // "groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource);;Argument[0];groovy", + { String script = request.getParameter("script"); final GroovyClassLoader classLoader = new GroovyClassLoader(); GroovyCodeSource gcs = new GroovyCodeSource(script, "test", "Test"); - Class groovy = classLoader.parseClass(gcs); // $hasGroovyInjection - GroovyObject groovyObj = (GroovyObject) groovy.newInstance(); - } catch (Exception e) { - // Ignore + classLoader.parseClass(gcs); // $hasGroovyInjection + } + // "groovy.lang;GroovyClassLoader;false;parseClass;(GroovyCodeSource,boolean);;Argument[0];groovy", + { + String script = request.getParameter("script"); + final GroovyClassLoader classLoader = new GroovyClassLoader(); + GroovyCodeSource gcs = new GroovyCodeSource(script, "test", "Test"); + classLoader.parseClass(gcs, true); // $hasGroovyInjection + } + // "groovy.lang;GroovyClassLoader;false;parseClass;(InputStream,String);;Argument[0];groovy", + { + String script = request.getParameter("script"); + final GroovyClassLoader classLoader = new GroovyClassLoader(); + classLoader.parseClass(new ByteArrayInputStream(script.getBytes()), "test"); // $hasGroovyInjection + } + // "groovy.lang;GroovyClassLoader;false;parseClass;(Reader,String);;Argument[0];groovy", + { + String script = request.getParameter("script"); + final GroovyClassLoader classLoader = new GroovyClassLoader(); + classLoader.parseClass(new StringReader(script), "test"); // $hasGroovyInjection + } + // "groovy.lang;GroovyClassLoader;false;parseClass;(String);;Argument[0];groovy", + { + String script = request.getParameter("script"); + final GroovyClassLoader classLoader = new GroovyClassLoader(); + classLoader.parseClass(script); // $hasGroovyInjection + } + // "groovy.lang;GroovyClassLoader;false;parseClass;(String,String);;Argument[0];groovy", + { + String script = request.getParameter("script"); + final GroovyClassLoader classLoader = new GroovyClassLoader(); + classLoader.parseClass(script, "test"); // $hasGroovyInjection } } } diff --git a/java/ql/test/stubs/groovy-all-3.0.7/groovy/lang/GroovyClassLoader.java b/java/ql/test/stubs/groovy-all-3.0.7/groovy/lang/GroovyClassLoader.java index aed97667742..2bc37d571e8 100644 --- a/java/ql/test/stubs/groovy-all-3.0.7/groovy/lang/GroovyClassLoader.java +++ b/java/ql/test/stubs/groovy-all-3.0.7/groovy/lang/GroovyClassLoader.java @@ -1,32 +1,287 @@ /* - * 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 + * 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 + * 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. + */ +/* + * @todo multi threaded compiling of the same class but with different roots for compilation... T1 + * compiles A, which uses B, T2 compiles B... mark A and B as parsed and then synchronize + * compilation. Problems: How to synchronize? How to get error messages? * - * 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 groovy.lang; -public class GroovyClassLoader { +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.Reader; +import java.net.URL; +import java.net.URLClassLoader; +import java.util.Enumeration; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.control.CompilationFailedException; +import org.codehaus.groovy.control.CompilerConfiguration; + +public class GroovyClassLoader extends URLClassLoader { public GroovyClassLoader() { + super(null); } - public Class parseClass(String text) { + public GroovyClassLoader(ClassLoader loader) { + super(null); + } + + public GroovyClassLoader(GroovyClassLoader parent) { + super(null); + } + + public GroovyClassLoader(ClassLoader parent, CompilerConfiguration config, + boolean useConfigurationClasspath) { + super(null); + } + + public GroovyClassLoader(ClassLoader loader, CompilerConfiguration config) { + super(null); + } + + + public Class defineClass(ClassNode classNode, String file, String newCodeBase) { return null; } - public Class parseClass(GroovyCodeSource gcs) { + public boolean hasCompatibleConfiguration(CompilerConfiguration config) { + return false; + } + + public Class parseClass(File file) throws CompilationFailedException, IOException { return null; } + + public Class parseClass(final String text, final String fileName) + throws CompilationFailedException { + return null; + } + + public Class parseClass(String text) throws CompilationFailedException { + return null; + } + + public synchronized String generateScriptName() { + return null; + } + + public Class parseClass(final Reader reader, final String fileName) + throws CompilationFailedException { + return null; + } + + public Class parseClass(final InputStream in, final String fileName) + throws CompilationFailedException { + return null; + } + + public Class parseClass(GroovyCodeSource codeSource) throws CompilationFailedException { + return null; + } + + public Class parseClass(final GroovyCodeSource codeSource, boolean shouldCacheSource) + throws CompilationFailedException { + return null; + } + + public static class InnerLoader extends GroovyClassLoader { + public InnerLoader(GroovyClassLoader delegate) {} + + @Override + public void addClasspath(String path) {} + + @Override + public void clearCache() {} + + @Override + public URL findResource(String name) { + return null; + } + + @Override + public Enumeration findResources(String name) throws IOException { + return null; + } + + @Override + public Class[] getLoadedClasses() { + return null; + } + + @Override + public URL getResource(String name) { + return null; + } + + @Override + public InputStream getResourceAsStream(String name) { + return null; + } + + @Override + public URL[] getURLs() { + return null; + } + + @Override + public Class loadClass(String name, boolean lookupScriptFiles, + boolean preferClassOverScript, boolean resolve) + throws ClassNotFoundException, CompilationFailedException { + return null; + } + + @Override + public Class parseClass(GroovyCodeSource codeSource, boolean shouldCache) + throws CompilationFailedException { + return null; + } + + @Override + public void addURL(URL url) {} + + @Override + public Class defineClass(ClassNode classNode, String file, String newCodeBase) { + return null; + } + + @Override + public Class parseClass(File file) throws CompilationFailedException, IOException { + return null; + } + + @Override + public Class parseClass(String text, String fileName) throws CompilationFailedException { + return null; + } + + @Override + public Class parseClass(String text) throws CompilationFailedException { + return null; + } + + @Override + public String generateScriptName() { + return null; + } + + @Override + public Class parseClass(Reader reader, String fileName) throws CompilationFailedException { + return null; + } + + @Override + public Class parseClass(InputStream in, String fileName) throws CompilationFailedException { + return null; + } + + @Override + public Class parseClass(GroovyCodeSource codeSource) throws CompilationFailedException { + return null; + } + + @Override + public Class defineClass(String name, byte[] b) { + return null; + } + + @Override + public Class loadClass(String name, boolean lookupScriptFiles, + boolean preferClassOverScript) + throws ClassNotFoundException, CompilationFailedException { + return null; + } + + @Override + public void setShouldRecompile(Boolean mode) {} + + @Override + public Boolean isShouldRecompile() { + return null; + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return null; + } + + @Override + public Enumeration getResources(String name) throws IOException { + return null; + } + + @Override + public void setDefaultAssertionStatus(boolean enabled) {} + + @Override + public void setPackageAssertionStatus(String packageName, boolean enabled) {} + + @Override + public void setClassAssertionStatus(String className, boolean enabled) {} + + @Override + public void clearAssertionStatus() {} + + @Override + public void close() throws IOException {} + + public long getTimeStamp() { + return 0; + } + + } + + public Class defineClass(String name, byte[] b) { + return null; + } + + public Class loadClass(final String name, boolean lookupScriptFiles, + boolean preferClassOverScript) + throws ClassNotFoundException, CompilationFailedException { + return null; + } + + public void addURL(URL url) {} + + public void setShouldRecompile(Boolean mode) {} + + public Boolean isShouldRecompile() { + return null; + } + + public Class loadClass(final String name, boolean lookupScriptFiles, + boolean preferClassOverScript, boolean resolve) + throws ClassNotFoundException, CompilationFailedException { + return null; + } + + @Override + public Class loadClass(String name) throws ClassNotFoundException { + return null; + } + + public void addClasspath(final String path) {} + + public Class[] getLoadedClasses() { + return null; + } + + public void clearCache() {} + + @Override + public void close() throws IOException {} + } diff --git a/java/ql/test/stubs/groovy-all-3.0.7/org/codehaus/groovy/ast/ClassNode.java b/java/ql/test/stubs/groovy-all-3.0.7/org/codehaus/groovy/ast/ClassNode.java new file mode 100644 index 00000000000..01b2bb983d3 --- /dev/null +++ b/java/ql/test/stubs/groovy-all-3.0.7/org/codehaus/groovy/ast/ClassNode.java @@ -0,0 +1,252 @@ +/* + * 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.codehaus.groovy.ast; + +import java.util.List; +import java.util.Set; + +public class ClassNode { + + public ClassNode redirect() { + return null; + } + + public boolean isRedirectNode() { + return false; + } + + public void setRedirect(ClassNode node) {} + + public ClassNode makeArray() { + return null; + } + + public boolean isPrimaryClassNode() { + return false; + } + + public ClassNode(Class c) {} + + + public boolean isSyntheticPublic() { + return false; + } + + public void setSyntheticPublic(boolean syntheticPublic) {} + + public ClassNode(String name, int modifiers, ClassNode superClass) {} + + + public void setSuperClass(ClassNode superClass) {} + + public ClassNode[] getInterfaces() { + return null; + } + + public void setInterfaces(ClassNode[] interfaces) {} + + public Set getAllInterfaces() { + return null; + } + + public String getName() { + return null; + } + + public String getUnresolvedName() { + return null; + } + + public String setName(String name) { + return null; + } + + public int getModifiers() { + return 0; + } + + public void setModifiers(int modifiers) {} + + public boolean hasProperty(String name) { + return false; + } + + public void addInterface(ClassNode type) {} + + public boolean equals(Object that) { + return false; + } + + public int hashCode() { + return 0; + } + + + public ClassNode getOuterClass() { + return null; + } + + public List getOuterClasses() { + return null; + } + + + + public boolean isDerivedFrom(ClassNode type) { + return false; + } + + public boolean isDerivedFromGroovyObject() { + return false; + } + + public boolean implementsAnyInterfaces(ClassNode... classNodes) { + return false; + } + + public boolean implementsInterface(ClassNode classNode) { + return false; + } + + public boolean declaresAnyInterfaces(ClassNode... classNodes) { + return false; + } + + public boolean declaresInterface(ClassNode classNode) { + return false; + } + + public ClassNode getSuperClass() { + return null; + } + + public ClassNode getUnresolvedSuperClass() { + return null; + } + + public ClassNode getUnresolvedSuperClass(boolean useRedirect) { + return null; + } + + public void setUnresolvedSuperClass(ClassNode superClass) {} + + public ClassNode[] getUnresolvedInterfaces() { + return null; + } + + public ClassNode[] getUnresolvedInterfaces(boolean useRedirect) { + return null; + } + + + + public String getPackageName() { + return null; + } + + public String getNameWithoutPackage() { + return null; + } + + + public boolean isStaticClass() { + return false; + } + + public void setStaticClass(boolean staticClass) {} + + public boolean isScriptBody() { + return false; + } + + public void setScriptBody(boolean scriptBody) {} + + public boolean isScript() { + return false; + } + + public void setScript(boolean script) {} + + public String toString() { + return null; + } + + public String toString(boolean showRedirect) { + return null; + } + + public boolean isInterface() { + return false; + } + + public boolean isAbstract() { + return false; + } + + public boolean isResolved() { + return false; + } + + public boolean isArray() { + return false; + } + + public ClassNode getComponentType() { + return null; + } + + public Class getTypeClass() { + return null; + } + + public boolean hasPackageName() { + return false; + } + + public void setAnnotated(boolean annotated) {} + + public boolean isAnnotated() { + return false; + } + + public void setGenericsPlaceHolder(boolean placeholder) {} + + public boolean isGenericsPlaceHolder() { + return false; + } + + public boolean isUsingGenerics() { + return false; + } + + public void setUsingGenerics(boolean usesGenerics) {} + + public ClassNode getPlainNodeReference() { + return null; + } + + public boolean isAnnotationDefinition() { + return false; + } + + public void renameField(String oldName, String newName) {} + + public void removeField(String oldName) {} + + public boolean isEnum() { + return false; + } + +}