From 534de6d8ae8a241562ffae002a96e40c1ae0b015 Mon Sep 17 00:00:00 2001 From: Maurizio Cimadamore Date: Thu, 25 May 2023 15:22:27 +0000 Subject: [PATCH] 8300491: SymbolLookup::libraryLookup accepts strings with terminators Reviewed-by: psandoz --- .../classes/java/lang/foreign/SymbolLookup.java | 6 ++++++ .../jdk/internal/foreign/SystemLookup.java | 2 ++ .../share/classes/jdk/internal/foreign/Utils.java | 4 ++++ test/jdk/java/foreign/LibraryLookupTest.java | 15 +++++++++++++++ test/jdk/java/foreign/StdLibTest.java | 5 +++++ .../java/foreign/TestClassLoaderFindNative.java | 6 ++++++ 6 files changed, 38 insertions(+) diff --git a/src/java.base/share/classes/java/lang/foreign/SymbolLookup.java b/src/java.base/share/classes/java/lang/foreign/SymbolLookup.java index e0ffc7adf59..f6ef8b23725 100644 --- a/src/java.base/share/classes/java/lang/foreign/SymbolLookup.java +++ b/src/java.base/share/classes/java/lang/foreign/SymbolLookup.java @@ -28,6 +28,7 @@ package java.lang.foreign; import jdk.internal.access.JavaLangAccess; import jdk.internal.access.SharedSecrets; import jdk.internal.foreign.MemorySessionImpl; +import jdk.internal.foreign.Utils; import jdk.internal.javac.PreviewFeature; import jdk.internal.loader.BuiltinClassLoader; import jdk.internal.loader.NativeLibrary; @@ -192,6 +193,7 @@ public interface SymbolLookup { } return name -> { Objects.requireNonNull(name); + if (Utils.containsNullChars(name)) return Optional.empty(); JavaLangAccess javaLangAccess = SharedSecrets.getJavaLangAccess(); // note: ClassLoader::findNative supports a null loader long addr = javaLangAccess.findNative(loader, name); @@ -229,6 +231,9 @@ public interface SymbolLookup { @CallerSensitive static SymbolLookup libraryLookup(String name, Arena arena) { Reflection.ensureNativeAccess(Reflection.getCallerClass(), SymbolLookup.class, "libraryLookup"); + if (Utils.containsNullChars(name)) { + throw new IllegalArgumentException("Cannot open library: " + name); + } return libraryLookup(name, RawNativeLibraries::load, arena); } @@ -279,6 +284,7 @@ public interface SymbolLookup { }); return name -> { Objects.requireNonNull(name); + if (Utils.containsNullChars(name)) return Optional.empty(); long addr = library.find(name); return addr == 0L ? Optional.empty() : diff --git a/src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java b/src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java index 94b9b2e6a23..dca7f50599d 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java +++ b/src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java @@ -95,6 +95,7 @@ public final class SystemLookup implements SymbolLookup { final SymbolLookup finalLookup = lookup; lookup = name -> { Objects.requireNonNull(name); + if (Utils.containsNullChars(name)) return Optional.empty(); return finalLookup.find(name).or(() -> fallbackLookup.apply(name)); }; } @@ -106,6 +107,7 @@ public final class SystemLookup implements SymbolLookup { NativeLibrary lib = loader.apply(RawNativeLibraries.newInstance(MethodHandles.lookup())); return name -> { Objects.requireNonNull(name); + if (Utils.containsNullChars(name)) return Optional.empty(); try { long addr = lib.lookup(name); return addr == 0 ? diff --git a/src/java.base/share/classes/jdk/internal/foreign/Utils.java b/src/java.base/share/classes/jdk/internal/foreign/Utils.java index 5289cab3f22..bb990540942 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/Utils.java +++ b/src/java.base/share/classes/jdk/internal/foreign/Utils.java @@ -251,4 +251,8 @@ public final class Utils { public static boolean isPowerOfTwo(long value) { return (value & (value - 1)) == 0L; } + + public static boolean containsNullChars(String s) { + return s.indexOf('\u0000') >= 0; + } } diff --git a/test/jdk/java/foreign/LibraryLookupTest.java b/test/jdk/java/foreign/LibraryLookupTest.java index ce62cb34d07..e64f959fbc9 100644 --- a/test/jdk/java/foreign/LibraryLookupTest.java +++ b/test/jdk/java/foreign/LibraryLookupTest.java @@ -67,6 +67,21 @@ public class LibraryLookupTest { callFunc(addr); } + @Test(expectedExceptions = IllegalArgumentException.class) + void testLoadLibraryBadName() { + try (Arena arena = Arena.ofConfined()) { + SymbolLookup.libraryLookup(LIB_PATH.toString() + "\u0000", arena); + } + } + + @Test + void testLoadLibraryBadLookupName() { + try (Arena arena = Arena.ofConfined()) { + SymbolLookup lookup = SymbolLookup.libraryLookup(LIB_PATH, arena); + assertTrue(lookup.find("inc\u0000foobar").isEmpty()); + } + } + private static MemorySegment loadLibrary(Arena session) { SymbolLookup lib = SymbolLookup.libraryLookup(LIB_PATH, session); MemorySegment addr = lib.find("inc").get(); diff --git a/test/jdk/java/foreign/StdLibTest.java b/test/jdk/java/foreign/StdLibTest.java index 86136f605b1..a66dc1e9b9c 100644 --- a/test/jdk/java/foreign/StdLibTest.java +++ b/test/jdk/java/foreign/StdLibTest.java @@ -134,6 +134,11 @@ public class StdLibTest extends NativeTestHelper { assertEquals(found, expected.length()); } + @Test + void testSystemLibraryBadLookupName() { + assertTrue(LINKER.defaultLookup().find("strlen\u0000foobar").isEmpty()); + } + static class StdLibHelper { final static MethodHandle strcat = abi.downcallHandle(abi.defaultLookup().find("strcat").get(), diff --git a/test/jdk/java/foreign/TestClassLoaderFindNative.java b/test/jdk/java/foreign/TestClassLoaderFindNative.java index 56a89a7476e..3f5fec0c195 100644 --- a/test/jdk/java/foreign/TestClassLoaderFindNative.java +++ b/test/jdk/java/foreign/TestClassLoaderFindNative.java @@ -28,6 +28,7 @@ * @run testng/othervm --enable-native-access=ALL-UNNAMED TestClassLoaderFindNative */ +import java.lang.foreign.Arena; import java.lang.foreign.MemorySegment; import java.lang.foreign.SymbolLookup; import org.testng.annotations.Test; @@ -60,4 +61,9 @@ public class TestClassLoaderFindNative { MemorySegment segment = SymbolLookup.loaderLookup().find("c").get().reinterpret(1); assertEquals(segment.get(JAVA_BYTE, 0), 42); } + + @Test + void testLoadLibraryBadLookupName() { + assertTrue(SymbolLookup.loaderLookup().find("f\u0000foobar").isEmpty()); + } }