Skip to content

Commit

Permalink
Bytecode inspection for Java-UDFs
Browse files Browse the repository at this point in the history
patch by Robert Stupp; reviewed by T Jake Luciani for CASSANDRA-9890
  • Loading branch information
snazy committed Jul 31, 2015
1 parent a22ce89 commit 1774eb9
Show file tree
Hide file tree
Showing 25 changed files with 1,356 additions and 7 deletions.
2 changes: 1 addition & 1 deletion CHANGES.txt
@@ -1,5 +1,5 @@
3.0
* Implement proper sandboxing for UDFs (CASSANDRA-9402)
* Implement proper sandboxing for UDFs (CASSANDRA-9402, 9890)
* Simplify (and unify) cleanup of compaction leftovers (CASSANDRA-7066)
* Allow extra schema definitions in cassandra-stress yaml (CASSANDRA-9850)
* Metrics should use up to date nomenclature (CASSANDRA-9448)
Expand Down
4 changes: 4 additions & 0 deletions NOTICE.txt
Expand Up @@ -79,3 +79,7 @@ Protocol buffers for varint encoding
https://developers.google.com/protocol-buffers/
Copyright 2008 Google Inc. All rights reserved.
BSD 3-clause

ASM
(http://asm.ow2.org/)
Copyright (c) 2000-2011 INRIA, France Telecom
Binary file added lib/asm-5.0.4.jar
Binary file not shown.
29 changes: 29 additions & 0 deletions lib/licenses/asm-5.0.4.txt
@@ -0,0 +1,29 @@
Copyright (c) 2000-2011 INRIA, France Telecom
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:

1. Redistributions of source code must retain the above copyright
notice, this list of conditions and the following disclaimer.

2. Redistributions in binary form must reproduce the above copyright
notice, this list of conditions and the following disclaimer in the
documentation and/or other materials provided with the distribution.

3. Neither the name of the copyright holders nor the names of its
contributors may be used to endorse or promote products derived from
this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
THE POSSIBILITY OF SUCH DAMAGE.
Expand Up @@ -38,9 +38,11 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.StringTokenizer;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -90,6 +92,8 @@ final class JavaBasedUDFunction extends UDFunction

private static final EcjTargetClassLoader targetClassLoader = new EcjTargetClassLoader();

private static final UDFByteCodeVerifier udfByteCodeVerifier = new UDFByteCodeVerifier();

private static final ProtectionDomain protectionDomain;

private static final IErrorHandlingPolicy errorHandlingPolicy = DefaultErrorHandlingPolicies.proceedWithAllProblems();
Expand All @@ -105,6 +109,24 @@ final class JavaBasedUDFunction extends UDFunction

static
{
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/Class", "forName");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/Class", "getClassLoader");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/Class", "getResource");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/Class", "getResourceAsStream");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "clearAssertionStatus");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getResource");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getResourceAsStream");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getResources");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getSystemClassLoader");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getSystemResource");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getSystemResourceAsStream");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "getSystemResources");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "loadClass");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "setClassAssertionStatus");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "setDefaultAssertionStatus");
udfByteCodeVerifier.addDisallowedMethodCall("java/lang/ClassLoader", "setPackageAssertionStatus");
udfByteCodeVerifier.addDisallowedMethodCall("java/nio/ByteBuffer", "allocateDirect");

Map<String, String> settings = new HashMap<>();
settings.put(CompilerOptions.OPTION_LineNumberAttribute,
CompilerOptions.GENERATE);
Expand Down Expand Up @@ -172,6 +194,8 @@ protected URLConnection openConnection(URL u)
String pkgName = BASE_PACKAGE + '.' + generateClassName(name, 'p');
String clsName = generateClassName(name, 'C');

String executeInternalName = generateClassName(name, 'x');

StringBuilder javaSourceBuilder = new StringBuilder();
int lineOffset = 1;
for (int i = 0; i < javaSourceTemplate.length; i++)
Expand Down Expand Up @@ -202,6 +226,9 @@ protected URLConnection openConnection(URL u)
case "return_type":
s = javaSourceName(javaReturnType);
break;
case "execute_internal_name":
s = executeInternalName;
break;
}
}

Expand Down Expand Up @@ -262,14 +289,31 @@ protected URLConnection openConnection(URL u)
throw new InvalidRequestException("Java source compilation failed:\n" + problems);
}

// Verify the UDF bytecode against use of probably dangerous code
Set<String> errors = udfByteCodeVerifier.verify(targetClassLoader.classData(targetClassName));
String validDeclare = "not allowed method declared: " + executeInternalName + '(';
String validCall = "call to " + targetClassName.replace('.', '/') + '.' + executeInternalName + "()";
for (Iterator<String> i = errors.iterator(); i.hasNext();)
{
String error = i.next();
// we generate a random name of the private, internal execute method, which is detected by the byte-code verifier
if (error.startsWith(validDeclare) || error.equals(validCall))
{
i.remove();
}
}
if (!errors.isEmpty())
throw new InvalidRequestException("Java UDF validation failed: " + errors);

// Load the class and create a new instance of it
Thread thread = Thread.currentThread();
ClassLoader orig = thread.getContextClassLoader();
try
{
thread.setContextClassLoader(UDFunction.udfClassLoader);
// Execute UDF intiialization from UDF class loader

Class cls = targetClassLoader.loadClass(targetClassName);
Class cls = Class.forName(targetClassName, false, targetClassLoader);

if (cls.getDeclaredMethods().length != 2 || cls.getDeclaredConstructors().length != 1)
throw new InvalidRequestException("Check your source to not define additional Java methods or constructors");
Expand Down Expand Up @@ -382,7 +426,7 @@ private static String generateArguments(Class<?>[] paramTypes, List<ColumnIdenti

private static String composeMethod(Class<?> type)
{
return (type.isPrimitive()) ? ("compose_" + type.getName()) : "compose";
return (type.isPrimitive()) ? ("super.compose_" + type.getName()) : "super.compose";
}

// Java source UDFs are a very simple compilation task, which allows us to let one class implement
Expand Down Expand Up @@ -578,6 +622,11 @@ void addClass(String className, byte[] classData)
classes.put(className, classData);
}

byte[] classData(String className)
{
return classes.get(className);
}

protected Class<?> findClass(String name) throws ClassNotFoundException
{
// remove the class binary - it's only used once - so it's wasting heap
Expand Down
215 changes: 215 additions & 0 deletions src/java/org/apache/cassandra/cql3/functions/UDFByteCodeVerifier.java
@@ -0,0 +1,215 @@
/*
* 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.cassandra.cql3.functions;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;

import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassVisitor;
import org.objectweb.asm.FieldVisitor;
import org.objectweb.asm.Handle;
import org.objectweb.asm.MethodVisitor;
import org.objectweb.asm.Opcodes;

/**
* Verifies Java UDF byte code.
* Checks for disallowed method calls (e.g. {@code Object.finalize()}),
* additional code in the constructor,
* use of {@code synchronized} blocks,
* too many methods.
*/
public final class UDFByteCodeVerifier
{

public static final String JAVA_UDF_NAME = JavaUDF.class.getName().replace('.', '/');
public static final String OBJECT_NAME = Object.class.getName().replace('.', '/');
public static final String CTOR_SIG = "(Lcom/datastax/driver/core/DataType;[Lcom/datastax/driver/core/DataType;)V";

private final Multimap<String, String> disallowedMethodCalls = HashMultimap.create();
private final List<String> disallowedPackages = new ArrayList<>();

public UDFByteCodeVerifier()
{
addDisallowedMethodCall(OBJECT_NAME, "clone");
addDisallowedMethodCall(OBJECT_NAME, "finalize");
addDisallowedMethodCall(OBJECT_NAME, "notify");
addDisallowedMethodCall(OBJECT_NAME, "notifyAll");
addDisallowedMethodCall(OBJECT_NAME, "wait");
}

public UDFByteCodeVerifier addDisallowedMethodCall(String clazz, String method)
{
disallowedMethodCalls.put(clazz, method);
return this;
}

public UDFByteCodeVerifier addDisallowedPackage(String pkg)
{
disallowedPackages.add(pkg);
return this;
}

public Set<String> verify(byte[] bytes)
{
Set<String> errors = new TreeSet<>(); // it's a TreeSet for unit tests
ClassVisitor classVisitor = new ClassVisitor(Opcodes.ASM5)
{
public FieldVisitor visitField(int access, String name, String desc, String signature, Object value)
{
errors.add("field declared: " + name);
return null;
}

public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions)
{
if ("<init>".equals(name) && CTOR_SIG.equals(desc))
{
if (Opcodes.ACC_PUBLIC != access)
errors.add("constructor not public");
// allowed constructor - JavaUDF(DataType returnDataType, DataType[] argDataTypes)
return new ConstructorVisitor(errors);
}
if ("executeImpl".equals(name) && "(ILjava/util/List;)Ljava/nio/ByteBuffer;".equals(desc))
{
if (Opcodes.ACC_PROTECTED != access)
errors.add("executeImpl not protected");
// the executeImpl method - ByteBuffer executeImpl(int protocolVersion, List<ByteBuffer> params)
return new ExecuteImplVisitor(errors);
}
if ("<clinit>".equals(name))
{
errors.add("static initializer declared");
}
else
{
errors.add("not allowed method declared: " + name + desc);
return new ExecuteImplVisitor(errors);
}
return null;
}

public void visit(int version, int access, String name, String signature, String superName, String[] interfaces)
{
if (!JAVA_UDF_NAME.equals(superName))
{
errors.add("class does not extend " + JavaUDF.class.getName());
}
if (access != (Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL | Opcodes.ACC_SUPER))
{
errors.add("class not public final");
}
super.visit(version, access, name, signature, superName, interfaces);
}

public void visitInnerClass(String name, String outerName, String innerName, int access)
{
errors.add("class declared as inner class");
super.visitInnerClass(name, outerName, innerName, access);
}
};

ClassReader classReader = new ClassReader(bytes);
classReader.accept(classVisitor, ClassReader.SKIP_DEBUG);

return errors;
}

private class ExecuteImplVisitor extends MethodVisitor
{
private final Set<String> errors;

ExecuteImplVisitor(Set<String> errors)
{
super(Opcodes.ASM5);
this.errors = errors;
}

public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf)
{
Collection<String> disallowed = disallowedMethodCalls.get(owner);
if (disallowed != null && disallowed.contains(name))
{
errors.add("call to " + name + "()");
}
if (!JAVA_UDF_NAME.equals(owner))
{
for (String pkg : disallowedPackages)
{
if (owner.startsWith(pkg))
errors.add("call to " + owner + '.' + name + "()");
}
}
super.visitMethodInsn(opcode, owner, name, desc, itf);
}

public void visitInsn(int opcode)
{
switch (opcode)
{
case Opcodes.MONITORENTER:
case Opcodes.MONITOREXIT:
errors.add("use of synchronized");
break;
}
super.visitInsn(opcode);
}
}

private static class ConstructorVisitor extends MethodVisitor
{
private final Set<String> errors;

ConstructorVisitor(Set<String> errors)
{
super(Opcodes.ASM5);
this.errors = errors;
}

public void visitInvokeDynamicInsn(String name, String desc, Handle bsm, Object... bsmArgs)
{
errors.add("Use of invalid method instruction in constructor");
super.visitInvokeDynamicInsn(name, desc, bsm, bsmArgs);
}

public void visitMethodInsn(int opcode, String owner, String name, String desc, boolean itf)
{
if (!(Opcodes.INVOKESPECIAL == opcode && JAVA_UDF_NAME.equals(owner) && "<init>".equals(name) && CTOR_SIG.equals(desc)))
{
errors.add("initializer declared");
}
super.visitMethodInsn(opcode, owner, name, desc, itf);
}

public void visitInsn(int opcode)
{
if (Opcodes.RETURN != opcode)
{
errors.add("initializer declared");
}
super.visitInsn(opcode);
}
}
}
Expand Up @@ -113,6 +113,7 @@ public abstract class UDFunction extends AbstractFunction implements ScalarFunct
"com/datastax/driver/core/Statement.class",
"com/datastax/driver/core/TimestampGenerator.class", // indirectly covers ServerSideTimestampGenerator + ThreadLocalMonotonicTimestampGenerator
"java/lang/Compiler.class",
"java/lang/InheritableThreadLocal.class",
"java/lang/Package.class",
"java/lang/Process.class",
"java/lang/ProcessBuilder.class",
Expand Down

0 comments on commit 1774eb9

Please sign in to comment.