Fix FFI panic unwinding, null pointer dereferences, and native buffer lifetime races

src/typst_core/src/lib.rs

@@ -87,18 +87,7 @@ impl Default for CompileResult {
     }
 }
 
-/// Creates a compiler that reads its document either from `input_path` or from `input_source`.
-///
-/// # Safety
-///
-/// `root`, `input_path`, `package_path` and `sys_inputs` must be null or NUL-terminated strings,
-/// `font_paths` must be null or point to `font_paths_len` such strings, and `input_source` must be
-/// null or point to `input_source_len` bytes. Unlike the others, the source is passed with an
-/// explicit length and may contain NUL bytes. All of them need only stay valid for the duration of
-/// the call. The returned compiler is owned by the caller and must be released with
-/// [`free_compiler`].
-#[unsafe(no_mangle)]
-pub unsafe extern "C" fn create_compiler(
+unsafe fn create_compiler_inner(
     root: *const c_char,
     input_path: *const c_char,
     input_source: *const u8,
@@ -139,12 +128,18 @@ pub unsafe extern "C" fn create_compiler(
     // which means an empty document.
     let input_content = if input_source.is_null() {
         None
+    } else if input_source_len == 0 {
+        Some(String::new())
     } else {
         let bytes = unsafe { std::slice::from_raw_parts(input_source, input_source_len) };
         Some(String::from_utf8_lossy(bytes).into_owned())
     };
 
-    let sys_inputs_str = unsafe { CStr::from_ptr(sys_inputs).to_str().unwrap_or("{}") };
+    let sys_inputs_str = if sys_inputs.is_null() {
+        "{}"
+    } else {
+        unsafe { CStr::from_ptr(sys_inputs).to_str().unwrap_or("{}") }
+    };
 
     let font_paths_vec: Vec<PathBuf> = unsafe {
         let slice: &[*const c_char] = if font_paths.is_null() || font_paths_len == 0 {
@@ -155,7 +150,12 @@ pub unsafe extern "C" fn create_compiler(
 
         slice
             .iter()
-            .map(|&p| PathBuf::from(CStr::from_ptr(p).to_str().unwrap_or("")))
+            .copied()
+            .filter(|p| !p.is_null())
+            .filter_map(|p| {
+                let s = CStr::from_ptr(p).to_str().unwrap_or("");
+                (!s.is_empty()).then(|| PathBuf::from(s))
+            })
             .collect()
     };
 
@@ -187,6 +187,46 @@ pub unsafe extern "C" fn create_compiler(
     }
 }
 
+/// Creates a compiler that reads its document either from `input_path` or from `input_source`.
+///
+/// # Safety
+///
+/// `root`, `input_path`, `package_path` and `sys_inputs` must be null or NUL-terminated strings,
+/// `font_paths` must be null or point to `font_paths_len` such strings (each of which may also be
+/// null), and `input_source` must be null or point to `input_source_len` bytes. Unlike the others,
+/// the source is passed with an explicit length and may contain NUL bytes. All of them need only
+/// stay valid for the duration of the call. The returned compiler is owned by the caller and must
+/// be released with [`free_compiler`].
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn create_compiler(
+    root: *const c_char,
+    input_path: *const c_char,
+    input_source: *const u8,
+    input_source_len: usize,
+    font_paths: *const *const c_char,
+    font_paths_len: usize,
+    package_path: *const c_char,
+    sys_inputs: *const c_char,
+    ignore_system_fonts: bool,
+    ignore_system_packages: bool,
+) -> *mut Compiler {
+    std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| unsafe {
+        create_compiler_inner(
+            root,
+            input_path,
+            input_source,
+            input_source_len,
+            font_paths,
+            font_paths_len,
+            package_path,
+            sys_inputs,
+            ignore_system_fonts,
+            ignore_system_packages,
+        )
+    }))
+    .unwrap_or(ptr::null_mut())
+}
+
 /// Releases a compiler created by [`create_compiler`]. A null pointer is ignored.
 ///
 /// Results previously returned by [`compile`] are unaffected: they own their memory and stay valid.
@@ -197,11 +237,13 @@ pub unsafe extern "C" fn create_compiler(
 /// freed, and no other thread may be using it.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn free_compiler(compiler: *mut Compiler) {
-    if !compiler.is_null() {
-        unsafe {
-            let _ = Box::from_raw(compiler);
+    let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        if !compiler.is_null() {
+            unsafe {
+                let _ = Box::from_raw(compiler);
+            }
         }
-    }
+    }));
 }
 
 /// Replaces the `sys.inputs` dictionary the next compilation will see. Returns `false` if the
@@ -213,26 +255,26 @@ pub unsafe extern "C" fn free_compiler(compiler: *mut Compiler) {
 /// null or a NUL-terminated JSON object that stays valid for the duration of the call.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn set_sys_inputs(compiler: *mut Compiler, sys_inputs: *const c_char) -> bool {
-    if compiler.is_null() {
-        return false;
-    }
-    let compiler = unsafe { &mut *compiler };
+    std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        if compiler.is_null() {
+            return false;
+        }
+        let compiler = unsafe { &mut *compiler };
 
-    let sys_inputs_str = if sys_inputs.is_null() {
-        "{}"
-    } else {
-        unsafe { CStr::from_ptr(sys_inputs).to_str().unwrap_or("{}") }
-    };
+        let sys_inputs_str = if sys_inputs.is_null() {
+            "{}"
+        } else {
+            unsafe { CStr::from_ptr(sys_inputs).to_str().unwrap_or("{}") }
+        };
 
-    let inputs: Dict = match serde_json::from_str(sys_inputs_str) {
-        Ok(d) => d,
-        Err(_) => return false,
-    };
+        let inputs: Dict = match serde_json::from_str(sys_inputs_str) {
+            Ok(d) => d,
+            Err(_) => return false,
+        };
 
-    match compiler.0.set_inputs(inputs) {
-        Ok(_) => true,
-        Err(_) => false,
-    }
+        compiler.0.set_inputs(inputs).is_ok()
+    }))
+    .unwrap_or(false)
 }
 
 fn compile_inner(
@@ -488,7 +530,7 @@ pub unsafe extern "C" fn compile(
 /// result frees the same allocations twice.
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn free_compile_result(result: CompileResult) {
-    unsafe {
+    let _ = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| unsafe {
         if !result.buffers.is_null() {
             let buffers = Box::from_raw(std::ptr::slice_from_raw_parts_mut(
                 result.buffers,
@@ -510,7 +552,7 @@ pub unsafe extern "C" fn free_compile_result(result: CompileResult) {
         if !result.error_ptr.is_null() {
             free_raw_slice(result.error_ptr, result.error_len);
         }
-    }
+    }));
 }
 
 /// How many evictions a cached result survives without being used, when the cache is trimmed after
@@ -523,7 +565,7 @@ const DEFAULT_MAX_CACHE_AGE: usize = 10;
 /// still holding.
 #[unsafe(no_mangle)]
 pub extern "C" fn reset_world() {
-    comemo::evict(DEFAULT_MAX_CACHE_AGE);
+    let _ = std::panic::catch_unwind(|| comemo::evict(DEFAULT_MAX_CACHE_AGE));
 }
 
 /// Clears the process-global incremental compilation cache completely.

src/typst_core/tests/compile.rs

@@ -197,4 +197,31 @@ fn the_cache_can_be_cleared_while_other_threads_compile() {
     }
 
     assert!(evictions > 0, "no eviction overlapped the compilations");
+}
+
+#[test]
+fn create_compiler_accepts_null_sys_inputs_and_null_font_path_entries() {
+    let source = b"= Null pointer resilience";
+    let font_paths: [*const c_char; 2] = [std::ptr::null(), std::ptr::null()];
+
+    let compiler = unsafe {
+        create_compiler(
+            std::ptr::null(),
+            std::ptr::null(),
+            source.as_ptr(),
+            source.len(),
+            font_paths.as_ptr(),
+            font_paths.len(),
+            std::ptr::null(),
+            std::ptr::null(),
+            true,
+            true,
+        )
+    };
+    assert!(!compiler.is_null(), "failed to create compiler with null sys_inputs and font_paths");
+
+    let pdf = compile_to_pdf(compiler);
+    assert!(pdf.starts_with(b"%PDF-"));
+
+    unsafe { free_compiler(compiler) };
 }

src/typstsharp.tests/Tests.cs

@@ -13,7 +13,7 @@
     public async Task BasicSource()
     {
         var compiler = TypstCompiler.FromSource("Hello World 2");
         var result = compiler.Compile().Buffers[0];
         var plainText = GetPlainText(result);
         await Assert.That(plainText).Contains("World 2");
     }
@@ -384,7 +384,7 @@
     {
         // Reported as #9
         var compiler = TypstCompiler.FromSource("= Hello world’s");
         var result = compiler.Compile().Buffers[0];
         var plainText = GetPlainText(result);
         await Assert.That(plainText).Contains("Hello world’s");
     }
@@ -635,7 +635,7 @@
             packagePath: packages.Path,
             includeSystemPackages: false);
 
         var plainText = GetPlainText(compiler.Compile().Buffers[0]);
 
         await Assert.That(plainText).Contains("Hello from a bundled package");
     }
@@ -1410,6 +1410,98 @@
         await Assert.That(document.Warnings.Count).IsEqualTo(0);
     }
 
+    [Test]
+    public async Task FontPathsWithNullOrEmptyEntriesAreIgnoredSafely()
+    {
+        using var compiler = TypstCompiler.FromSource(
+            "= Null font path entry",
+            fonts: new Fonts(IncludeSystemFonts: false, FontPaths: [null!, ""]));
+
+        var plainText = GetPlainText(compiler.CompilePdf());
+        await Assert.That(plainText).Contains("Null font path entry");
+    }
+
+    [Test]
+    public async Task DisposingCompilerConcurrentlyIsSafe()
+    {
+        var compiler = TypstCompiler.FromSource("= Concurrent dispose");
+        using var barrier = new Barrier(8);
+
+        var tasks = Enumerable.Range(0, 8).Select(_ => Task.Run(() =>
+        {
+            barrier.SignalAndWait();
+            compiler.Dispose();
+        })).ToArray();
+
+        await Task.WhenAll(tasks);
+
+        await Assert.That(() => compiler.CompilePdf()).Throws<ObjectDisposedException>();
+        await Assert.That(() => compiler.SetSysInputs(new Dictionary<string, string>())).Throws<ObjectDisposedException>();
+    }
+
+    /// <summary>
+    /// If a document is disposed while an asynchronous write still has the native buffer pinned
+    /// (for instance when a cancellation token unwinds a caller's using scope while OS overlapped
+    /// I/O is still in flight), freeing must wait until the pin and lease are released.
+    /// </summary>
+    [Test]
+    public async Task DisposingDocumentWhileBufferIsPinnedDefersFreeUntilUnpinned()
+    {
+        using var compiler = TypstCompiler.FromSource("= Pinned across disposal\n\n#lorem(100)");
+        var document = compiler.CompileToDocument();
+        var expected = document.GetOutputBytes();
+
+        var pinnedSignal = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        var disposedSignal = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
+        var sink = new PinningAsyncStream(pinnedSignal, disposedSignal);
+
+        var writeTask = document.CopyOutputToAsync(sink).AsTask();
+
+        await pinnedSignal.Task;
+
+        // Dispose the document while the stream still holds a MemoryHandle pin over its native buffer.
+        document.Dispose();
+
+        // New operations must immediately observe that the document is disposed.
+        await Assert.That(() => document.GetOutputBytes()).Throws<ObjectDisposedException>();
+
+        // Let the in-flight pinned write read from the pointer and then unpin.
+        disposedSignal.SetResult();
+        var captured = await writeTask.ContinueWith(_ => sink.CapturedBytes);
+
+        await Assert.That(captured.SequenceEqual(expected)).IsTrue();
+    }
+
+    private sealed class PinningAsyncStream(
+        TaskCompletionSource pinnedSignal,
+        TaskCompletionSource disposedSignal) : Stream
+    {
+        public byte[] CapturedBytes { get; private set; } = [];
+
+        public override async ValueTask WriteAsync(ReadOnlyMemory<byte> buffer, CancellationToken cancellationToken = default)
+        {
+            using var handle = buffer.Pin();
+            pinnedSignal.SetResult();
+            await disposedSignal.Task.ConfigureAwait(false);
+
+            CapturedBytes = CopyFromHandle(handle, buffer.Length);
+        }
+
+        private static unsafe byte[] CopyFromHandle(MemoryHandle handle, int length) =>
+            new ReadOnlySpan<byte>(handle.Pointer, length).ToArray();
+
+        public override bool CanRead => false;
+        public override bool CanSeek => false;
+        public override bool CanWrite => true;
+        public override long Length => throw new NotSupportedException();
+        public override long Position { get => throw new NotSupportedException(); set => throw new NotSupportedException(); }
+        public override void Flush() { }
+        public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException();
+        public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException();
+        public override void SetLength(long value) => throw new NotSupportedException();
+        public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException();
+    }
+
     private const string TwoPageSource = """
                                          First page
                                          #pagebreak()

src/typstsharp.tests/typstsharp.tests.csproj

@@ -3,6 +3,7 @@
   <PropertyGroup>
     <ImplicitUsings>enable</ImplicitUsings>
     <Nullable>enable</Nullable>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <OutputType>Exe</OutputType>
     <TargetFramework>net10.0</TargetFramework>
     <TestingPlatformDotnetTestSupport>true</TestingPlatformDotnetTestSupport>

src/typstsharp/Bindings.g.cs

@@ -24,11 +24,11 @@ internal static unsafe partial class NativeMethods
         ///  # Safety
         ///
         ///  `root`, `input_path`, `package_path` and `sys_inputs` must be null or NUL-terminated strings,
-        ///  `font_paths` must be null or point to `font_paths_len` such strings, and `input_source` must be
-        ///  null or point to `input_source_len` bytes. Unlike the others, the source is passed with an
-        ///  explicit length and may contain NUL bytes. All of them need only stay valid for the duration of
-        ///  the call. The returned compiler is owned by the caller and must be released with
-        ///  [`free_compiler`].
+        ///  `font_paths` must be null or point to `font_paths_len` such strings (each of which may also be
+        ///  null), and `input_source` must be null or point to `input_source_len` bytes. Unlike the others,
+        ///  the source is passed with an explicit length and may contain NUL bytes. All of them need only
+        ///  stay valid for the duration of the call. The returned compiler is owned by the caller and must
+        ///  be released with [`free_compiler`].
         /// </summary>
         [DllImport(__DllName, EntryPoint = "create_compiler", CallingConvention = CallingConvention.Cdecl, ExactSpelling = true)]
         internal static extern Compiler* create_compiler(byte* root, byte* input_path, byte* input_source, nuint input_source_len, byte** font_paths, nuint font_paths_len, byte* package_path, byte* sys_inputs, [MarshalAs(UnmanagedType.U1)] bool ignore_system_fonts, [MarshalAs(UnmanagedType.U1)] bool ignore_system_packages);