From 6f03ea8dc54a6e287d9e6c7b5cc1d26cc0f455b9 Mon Sep 17 00:00:00 2001 From: Maksim Pelevin Date: Fri, 29 Jul 2022 17:16:12 +0300 Subject: [PATCH 1/2] Add SecurityManager support to block suspicious code #622 --- .../framework/plugin/api/UtExecutionResult.kt | 4 + .../main/kotlin/org/utbot/engine/Resolver.kt | 13 +- .../constructor/tree/CgMethodConstructor.kt | 14 ++ .../concrete/UtExecutionInstrumentation.kt | 5 + .../instrumentation/InvokeInstrumentation.kt | 5 +- .../instrumentation/process/ChildProcess.kt | 7 + .../utbot/instrumentation/process/Security.kt | 144 ++++++++++++++++++ .../instrumentation/security/SecurityTest.kt | 59 +++++++ .../kotlin/org/utbot/summary/TagGenerator.kt | 5 +- 9 files changed, 247 insertions(+), 9 deletions(-) create mode 100644 utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt create mode 100644 utbot-instrumentation/src/test/kotlin/org/utbot/instrumentation/security/SecurityTest.kt diff --git a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/UtExecutionResult.kt b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/UtExecutionResult.kt index 45d5272ec5..dfb4235b37 100644 --- a/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/UtExecutionResult.kt +++ b/utbot-framework-api/src/main/kotlin/org/utbot/framework/plugin/api/UtExecutionResult.kt @@ -17,6 +17,10 @@ data class UtOverflowFailure( override val exception: Throwable, ) : UtExecutionFailure() +data class UtSandboxFailure( + override val exception: Throwable +) : UtExecutionFailure() + /** * unexpectedFail (when exceptions such as NPE, IOBE, etc. appear, but not thrown by a user, applies both for function under test and nested calls ) * expectedCheckedThrow (when function under test or nested call explicitly says that checked exception could be thrown and throws it) diff --git a/utbot-framework/src/main/kotlin/org/utbot/engine/Resolver.kt b/utbot-framework/src/main/kotlin/org/utbot/engine/Resolver.kt index 128e9f4c78..4cbd37f30e 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/engine/Resolver.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/engine/Resolver.kt @@ -72,6 +72,7 @@ import kotlin.math.min import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.persistentSetOf import org.utbot.framework.plugin.api.SYMBOLIC_NULL_ADDR +import org.utbot.framework.plugin.api.UtSandboxFailure import soot.ArrayType import soot.BooleanType import soot.ByteType @@ -88,6 +89,7 @@ import soot.SootClass import soot.SootField import soot.Type import soot.VoidType +import java.security.AccessControlException // hack const val MAX_LIST_SIZE = 10 @@ -371,12 +373,11 @@ class Resolver( return if (explicit) { UtExplicitlyThrownException(exception, inNestedMethod) } else { - // TODO SAT-1561 - val isOverflow = exception is ArithmeticException && exception.message?.contains("overflow") == true - if (isOverflow) { - UtOverflowFailure(exception) - } else { - UtImplicitlyThrownException(exception, inNestedMethod) + when { + // TODO SAT-1561 + exception is ArithmeticException && exception.message?.contains("overflow") == true -> UtOverflowFailure(exception) + exception is AccessControlException -> UtSandboxFailure(exception) + else -> UtImplicitlyThrownException(exception, inNestedMethod) } } } diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt index f99fb84b46..56fe6caf80 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/codegen/model/constructor/tree/CgMethodConstructor.kt @@ -106,6 +106,7 @@ import org.utbot.framework.plugin.api.UtNewInstanceInstrumentation import org.utbot.framework.plugin.api.UtNullModel import org.utbot.framework.plugin.api.UtPrimitiveModel import org.utbot.framework.plugin.api.UtReferenceModel +import org.utbot.framework.plugin.api.UtSandboxFailure import org.utbot.framework.plugin.api.UtStaticMethodInstrumentation import org.utbot.framework.plugin.api.UtSymbolicExecution import org.utbot.framework.plugin.api.UtTimeoutException @@ -141,6 +142,7 @@ import org.utbot.framework.plugin.api.util.wrapIfPrimitive import org.utbot.framework.util.isUnit import org.utbot.summary.SummarySentenceConstants.TAB import java.lang.reflect.InvocationTargetException +import java.security.AccessControlException import java.lang.reflect.ParameterizedType import kotlin.reflect.jvm.javaType @@ -351,6 +353,11 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c methodType = CRASH writeWarningAboutCrash() } + is AccessControlException -> { + methodType = CRASH + writeWarningAboutFailureTest(exception) + return + } else -> { methodType = FAILING writeWarningAboutFailureTest(exception) @@ -361,6 +368,7 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c } private fun shouldTestPassWithException(execution: UtExecution, exception: Throwable): Boolean { + if (exception is AccessControlException) return false // tests with timeout or crash should be processed differently if (exception is TimeoutException || exception is ConcreteExecutionFailureException) return false @@ -1533,6 +1541,12 @@ internal class CgMethodConstructor(val context: CgContext) : CgContextOwner by c ) } + if (result is UtSandboxFailure) { + testFrameworkManager.disableTestMethod( + "Disabled due to sandbox" + ) + } + val testMethod = buildTestMethod { name = methodName parameters = params diff --git a/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt b/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt index 09ac24c414..c43ae20ba5 100644 --- a/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt +++ b/utbot-framework/src/main/kotlin/org/utbot/framework/concrete/UtExecutionInstrumentation.kt @@ -20,6 +20,7 @@ import org.utbot.framework.plugin.api.UtInstrumentation import org.utbot.framework.plugin.api.UtMethod import org.utbot.framework.plugin.api.UtModel import org.utbot.framework.plugin.api.UtNewInstanceInstrumentation +import org.utbot.framework.plugin.api.UtSandboxFailure import org.utbot.framework.plugin.api.UtStaticMethodInstrumentation import org.utbot.framework.plugin.api.UtTimeoutException import org.utbot.framework.plugin.api.util.UtContext @@ -37,6 +38,7 @@ import org.utbot.instrumentation.instrumentation.et.ExplicitThrowInstruction import org.utbot.instrumentation.instrumentation.et.TraceHandler import org.utbot.instrumentation.instrumentation.instrumenter.Instrumenter import org.utbot.instrumentation.instrumentation.mock.MockClassVisitor +import java.security.AccessControlException import java.security.ProtectionDomain import java.util.IdentityHashMap import kotlin.reflect.jvm.javaMethod @@ -221,6 +223,9 @@ object UtExecutionInstrumentation : Instrumentation { if (exception is TimeoutException) { return UtTimeoutException(exception) } + if (exception is AccessControlException) { + return UtSandboxFailure(exception) + } val instrs = traceHandler.computeInstructionList() val isNested = if (instrs.isEmpty()) { false diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/InvokeInstrumentation.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/InvokeInstrumentation.kt index 229bd8452f..0fd8e5bb3d 100644 --- a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/InvokeInstrumentation.kt +++ b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/instrumentation/InvokeInstrumentation.kt @@ -2,6 +2,7 @@ package org.utbot.instrumentation.instrumentation import org.utbot.common.withAccessibility import org.utbot.framework.plugin.api.util.signature +import org.utbot.instrumentation.process.sandbox import java.lang.reflect.Constructor import java.lang.reflect.InvocationTargetException import java.lang.reflect.Method @@ -54,7 +55,7 @@ class InvokeInstrumentation : Instrumentation> { is Method -> withAccessibility { runCatching { - invoke(thisObject, *realArgs.toTypedArray()).let { + sandbox { invoke(thisObject, *realArgs.toTypedArray()) }.let { if (returnType != Void.TYPE) it else Unit } // invocation on method returning void will return null, so we replace it with Unit } @@ -63,7 +64,7 @@ class InvokeInstrumentation : Instrumentation> { is Constructor<*> -> withAccessibility { runCatching { - newInstance(*realArgs.toTypedArray()) + sandbox { newInstance(*realArgs.toTypedArray()) } } } diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt index 77e1d9a674..0d4531e804 100644 --- a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt +++ b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/ChildProcess.kt @@ -11,6 +11,7 @@ import java.io.File import java.io.OutputStream import java.io.PrintStream import java.net.URLClassLoader +import java.security.AllPermission import java.time.LocalDateTime import java.time.format.DateTimeFormatter import kotlin.system.exitProcess @@ -51,6 +52,12 @@ private val kryoHelper: KryoHelper = KryoHelper(System.`in`, System.`out`) * It should be compiled into separate jar file (child_process.jar) and be run with an agent (agent.jar) option. */ fun main() { + permissions { + // Enable all permissions for instrumentation. + // SecurityKt.sandbox() is used to restrict these permissions. + + AllPermission() + } + // We don't want user code to litter the standard output, so we redirect it. val tmpStream = PrintStream(object : OutputStream() { override fun write(b: Int) {} diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt new file mode 100644 index 0000000000..8e81e2054e --- /dev/null +++ b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt @@ -0,0 +1,144 @@ +@file:Suppress("JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE") + +package org.utbot.instrumentation.process + +import sun.security.provider.PolicyFile +import java.net.URI +import java.nio.file.Files +import java.nio.file.Paths +import java.security.AccessControlContext +import java.security.AccessController +import java.security.CodeSource +import java.security.Permission +import java.security.PermissionCollection +import java.security.Permissions +import java.security.Policy +import java.security.PrivilegedAction +import java.security.PrivilegedActionException +import java.security.ProtectionDomain +import java.security.cert.Certificate + +internal fun permissions(block: SimplePolicy.() -> Unit) { + val policy = Policy.getPolicy() + if (policy !is SimplePolicy) { + Policy.setPolicy(SimplePolicy(block)) + System.setSecurityManager(SecurityManager()) + } else { + policy.block() + } +} + +/** + * Run [block] in sandbox mode. + * + * When running in sandbox by default only necessary to instrumentation permissions are enabled. + * Other options are not enabled by default and rises [java.security.AccessControlException]. + * + * To add new permissions create and/or edit file "{user.home}/.utbot/sandbox.policy". + * + * For example to enable property reading (`System.getProperty("user.home")`): + * + * ``` + * grant { + * permission java.util.PropertyPermission "user.home", "read"; + * }; + * ``` + * Read more [about policy file and syntax](https://docs.oracle.com/javase/7/docs/technotes/guides/security/PolicyFiles.html#Examples) + */ +internal fun sandbox(block: () -> T): T { + val policyPath = Paths.get(System.getProperty("user.home"), ".utbot", "sandbox.policy") + return sandbox(policyPath.toUri()) { block() } +} + +internal fun sandbox(file: URI, block: () -> T): T { + val path = Paths.get(file) + val perms = mutableListOf() + val allCodeSource = CodeSource(null, emptyArray()) + if (Files.exists(path)) { + val policyFile = PolicyFile(file.toURL()) + val collection = policyFile.getPermissions(allCodeSource) + perms += collection.elements().toList() + } + return sandbox(perms, allCodeSource) { block() } +} + +internal fun sandbox(permission: List, cs: CodeSource, block: () -> T): T { + val perms = permission.fold(Permissions()) { acc, p -> acc.add(p); acc } + return sandbox(perms, cs) { block() } +} + +internal fun sandbox(perms: PermissionCollection, cs: CodeSource, block: () -> T): T { + val acc = AccessControlContext(arrayOf(ProtectionDomain(cs, perms))) + return try { + AccessController.doPrivileged(PrivilegedAction { block() }, acc) + } catch (e: PrivilegedActionException) { + throw e.exception + } +} + +/** + * This policy can add grant or denial rules for permissions. + * + * To add a grant permission use like this in any place: + * + * ``` + * permissions { + * + java.security.PropertyPolicy("user.home", "read,write") + * } + * ``` + * + * After first call [SecurityManager] is set with this policy + * + * To deny a permission: + * + * ``` + * permissions { + * - java.security.PropertyPolicy("user.home", "read,write") + * } + * ``` + * + * To delete all concrete permissions (if it was added before): + * + * ``` + * permissions { + * ! java.security.PropertyPolicy("user.home", "read,write") + * } + * ``` + * + * The last permission has priority. Enable all property read for "user.*", but forbid to read only "user.home": + * + * ``` + * permissions { + * + java.security.PropertyPolicy("user.*", "read,write") + * - java.security.PropertyPolicy("user.home", "read,write") + * } + * ``` + */ +internal class SimplePolicy(init: SimplePolicy.() -> Unit = {}) : Policy() { + sealed class Access(val permission: Permission) { + class Allow(permission: Permission) : Access(permission) + class Deny(permission: Permission) : Access(permission) + } + private var permissions = mutableListOf() + + init { apply(init) } + + operator fun Permission.unaryPlus() = permissions.add(Access.Allow(this)) + + operator fun Permission.unaryMinus() = permissions.add(Access.Deny(this)) + + operator fun Permission.not() = permissions.removeAll { it.permission == this } + + override fun getPermissions(codesource: CodeSource) = UNSUPPORTED_EMPTY_COLLECTION!! + override fun getPermissions(domain: ProtectionDomain) = UNSUPPORTED_EMPTY_COLLECTION!! + override fun implies(domain: ProtectionDomain, permission: Permission): Boolean { + // 0 means no info, < 0 is denied and > 0 is allowed + val result = permissions.lastOrNull { it.permission.implies(permission) }?.let { + when (it) { + is Access.Allow -> 1 + is Access.Deny -> -1 + } + } ?: 0 + return result > 0 + } +} \ No newline at end of file diff --git a/utbot-instrumentation/src/test/kotlin/org/utbot/instrumentation/security/SecurityTest.kt b/utbot-instrumentation/src/test/kotlin/org/utbot/instrumentation/security/SecurityTest.kt new file mode 100644 index 0000000000..517b26118c --- /dev/null +++ b/utbot-instrumentation/src/test/kotlin/org/utbot/instrumentation/security/SecurityTest.kt @@ -0,0 +1,59 @@ +package org.utbot.instrumentation.security + +import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows +import org.utbot.instrumentation.process.permissions +import org.utbot.instrumentation.process.sandbox +import java.lang.NullPointerException +import java.security.AccessControlException +import java.security.AllPermission +import java.security.BasicPermission +import java.security.CodeSource +import java.security.Permission +import java.security.cert.Certificate +import java.util.PropertyPermission + +class SecurityTest { + + @BeforeEach + fun init() { + permissions { + +AllPermission() + } + } + + @Test + fun `basic security works`() { + sandbox { + assertThrows { + System.getProperty("any") + } + } + } + + @Test + fun `basic permission works`() { + sandbox(listOf(PropertyPermission("java.version", "read")), CodeSource(null, emptyArray())) { + val result = System.getProperty("java.version") + assertNotNull(result) + assertThrows { + System.setProperty("any_random_value_key", "random") + } + } + } + + @Test + fun `null is ok`() { + val empty = object : BasicPermission("*") {} + val field = Permission::class.java.getDeclaredField("name") + field.isAccessible = true + field.set(empty, null) + val collection = empty.newPermissionCollection() + assertThrows { + collection.implies(empty) + } + } + +} \ No newline at end of file diff --git a/utbot-summary/src/main/kotlin/org/utbot/summary/TagGenerator.kt b/utbot-summary/src/main/kotlin/org/utbot/summary/TagGenerator.kt index b4bbcace54..9f7f4d9c92 100644 --- a/utbot-summary/src/main/kotlin/org/utbot/summary/TagGenerator.kt +++ b/utbot-summary/src/main/kotlin/org/utbot/summary/TagGenerator.kt @@ -9,6 +9,7 @@ import org.utbot.framework.plugin.api.UtExplicitlyThrownException import org.utbot.framework.plugin.api.UtImplicitlyThrownException import org.utbot.framework.plugin.api.UtOverflowFailure import org.utbot.framework.plugin.api.UtMethodTestSet +import org.utbot.framework.plugin.api.UtSandboxFailure import org.utbot.framework.plugin.api.UtTimeoutException import org.utbot.framework.plugin.api.util.isCheckedException import org.utbot.summary.UtSummarySettings.MIN_NUMBER_OF_EXECUTIONS_FOR_CLUSTERING @@ -140,7 +141,8 @@ enum class ClusterKind { EXPLICITLY_THROWN_UNCHECKED_EXCEPTIONS, OVERFLOWS, TIMEOUTS, - CRASH_SUITE; + CRASH_SUITE, + SECURITY; val displayName: String get() = toString().replace('_', ' ') } @@ -152,6 +154,7 @@ private fun UtExecutionResult.clusterKind() = when (this) { is UtOverflowFailure -> ClusterKind.OVERFLOWS is UtTimeoutException -> ClusterKind.TIMEOUTS is UtConcreteExecutionFailure -> ClusterKind.CRASH_SUITE + is UtSandboxFailure -> ClusterKind.SECURITY } /** From 8e27750cc4676408f593dcd04b141f62aeb2ea22 Mon Sep 17 00:00:00 2001 From: Maksim Pelevin Date: Mon, 22 Aug 2022 16:19:36 +0300 Subject: [PATCH 2/2] Fix mock-relative issues due to lack of permissions --- .../main/kotlin/org/utbot/instrumentation/process/Security.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt index 8e81e2054e..f94bfb06e6 100644 --- a/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt +++ b/utbot-instrumentation/src/main/kotlin/org/utbot/instrumentation/process/Security.kt @@ -52,7 +52,9 @@ internal fun sandbox(block: () -> T): T { internal fun sandbox(file: URI, block: () -> T): T { val path = Paths.get(file) - val perms = mutableListOf() + val perms = mutableListOf( + RuntimePermission("accessDeclaredMembers") + ) val allCodeSource = CodeSource(null, emptyArray()) if (Files.exists(path)) { val policyFile = PolicyFile(file.toURL())