fix: context buffer overflow (#21)

.gitignore

@@ -21,3 +21,7 @@ examples/**/rust_client/target/
 
 # Development plan (local only)
 PLAN.md
+
+# Compiled test binaries (extensionless executables)
+tests/test_*
+!tests/test_*.nim

ffi.nimble

@@ -24,6 +24,7 @@ task test, "Run all tests under --mm:orc and --mm:refc":
     exec "nim c -r " & flags & " tests/test_ffi_context.nim"
     exec "nim c -r " & flags & " tests/test_gc_compat.nim"
     exec "nim c -r " & flags & " tests/test_serial.nim"
+    exec "nim c -r " & flags & " tests/test_ctx_validation.nim"
 
 task test_alloc, "Run alloc unit tests under --mm:orc and --mm:refc":
   exec "nim c -r " & nimFlagsOrc & " tests/test_alloc.nim"

ffi/ffi_context.nim

@@ -2,7 +2,7 @@
 {.pragma: callback, cdecl, raises: [], gcsafe.}
 {.passc: "-fPIC".}
 
-import std/[options, atomics, os, net, locks, json, tables]
+import std/[options, atomics, os, net, locks, json, tables, sets]
 import chronicles, chronos, chronos/threadsync, taskpools/channels_spsc_single, results
 import ./ffi_types, ./ffi_thread_request, ./internal/ffi_macro, ./logging
 
@@ -41,6 +41,30 @@ var ffiCurrentCallbackState* {.threadvar.}: ptr FFICallbackState
 
 const git_version* {.strdefine.} = "n/a"
 
+var contextRegistry = initHashSet[pointer]()
+var contextRegistryLock: Lock
+contextRegistryLock.initLock()
+
+proc registerCtx(ctx: pointer) =
+  {.cast(gcsafe).}:
+    contextRegistryLock.acquire()
+    defer: contextRegistryLock.release()
+    contextRegistry.incl(ctx)
+
+proc unregisterCtx(ctx: pointer) =
+  {.cast(gcsafe).}:
+    contextRegistryLock.acquire()
+    defer: contextRegistryLock.release()
+    contextRegistry.excl(ctx)
+
+proc isValidCtx*(ctx: pointer): bool =
+  ## Returns true only if ctx was created by createFFIContext and not yet destroyed.
+  ## Rejects nil, offset-invalid, and dangling pointers at the API boundary.
+  {.cast(gcsafe).}:
+    contextRegistryLock.acquire()
+    defer: contextRegistryLock.release()
+    return contextRegistry.contains(ctx)
+
 template callEventCallback*(ctx: ptr FFIContext, eventName: string, body: untyped) =
   if isNil(ctx[].callbackState.callback):
     chronicles.error eventName & " - eventCallback is nil"
@@ -84,6 +108,9 @@ template dispatchFfiEvent*(eventName: string, body: untyped) =
 proc sendRequestToFFIThread*(
     ctx: ptr FFIContext, ffiRequest: ptr FFIThreadRequest, timeout = InfiniteDuration
 ): Result[void, string] =
+  if not isValidCtx(cast[pointer](ctx)):
+    deleteRequest(ffiRequest)
+    return err("ctx is not a valid FFI context")
   ctx.lock.acquire()
   # This lock is only necessary while we use a SP Channel and while the signalling
   # between threads assumes that there aren't concurrent requests.
@@ -336,6 +363,7 @@ proc createFFIContext*[T](): Result[ptr FFIContext[T], string] =
     joinThread(ctx.ffiThread)
     return err("failed to create the watchdog thread: " & getCurrentExceptionMsg())
 
+  registerCtx(cast[pointer](ctx))
   success = true
   return ok(ctx)
 
@@ -346,6 +374,7 @@ proc destroyFFIContext*[T](ctx: ptr FFIContext[T]): Result[void, string] =
   ## the thread will eventually exit on its own, but cleanup is skipped
   ## because the thread may still be touching ctx fields.
   const ThreadExitTimeout = 1500.milliseconds
+  unregisterCtx(cast[pointer](ctx))
 
   ctx.running.store(false)
 
@@ -380,8 +409,10 @@ proc destroyFFIContext*[T](ctx: ptr FFIContext[T]): Result[void, string] =
   return ok()
 
 template checkParams*(ctx: ptr FFIContext, callback: FFICallBack, userData: pointer) =
-  if not isNil(ctx):
-    ctx[].userData = userData
+  if not isValidCtx(cast[pointer](ctx)):
+    return RET_ERR
+
+  ctx[].userData = userData
 
   if isNil(callback):
     return RET_MISSING_CALLBACK

ffi/internal/ffi_macro.nim

@@ -569,8 +569,9 @@ macro ffiRaw*(prc: untyped): untyped =
   let ffiBody = newStmtList(
     quote do:
       initializeLibrary()
-      if not isNil(ctx):
-        ctx[].userData = userData
+      if not isValidCtx(cast[pointer](ctx)):
+        return RET_ERR
+      ctx[].userData = userData
       if isNil(callback):
         return RET_MISSING_CALLBACK
   )

tests/test_ctx_validation.nim

@@ -0,0 +1,41 @@
+import std/locks
+import unittest2
+import results
+import ../ffi
+
+type TestLib = object
+
+proc dummyCallback(
+    retCode: cint, msg: ptr cchar, len: csize_t, userData: pointer
+) {.cdecl, gcsafe, raises: [].} =
+  discard
+
+registerReqFFI(ValidationTestRequest, lib: ptr TestLib):
+  proc(): Future[Result[string, string]] {.async.} =
+    return ok("ok")
+
+suite "ctx pointer validation":
+  # BUG: sendRequestToFFIThread has no nil-check on ctx.
+  # checkParams / {.ffi.} generated code only guards against nil callback,
+  # not nil (or otherwise invalid) ctx.  Any caller — C or Nim — that passes
+  # a nil or offset-invalid ctx with a valid callback bypasses the only guard
+  # and reaches ctx.lock.acquire() where the nil/garbage dereference crashes.
+
+  test "nil ctx with valid callback should return an error, not crash":
+    # Reproduces the nil case: ctx=nil, callback=valid.
+    # Expected (after fix): sendRequestToFFIThread returns isErr().
+    # Actual (currently)  : SIGSEGV at ctx.lock.acquire() in sendRequestToFFIThread.
+    let nilCtx: ptr FFIContext[TestLib] = nil
+    let req = ValidationTestRequest.ffiNewReq(dummyCallback, nil)
+    let res = sendRequestToFFIThread(nilCtx, req)
+    check res.isErr()
+
+  test "invalid non-nil ctx (ctx+123 style) should return an error, not crash":
+    # Reproduces the offset-pointer case: a non-nil but invalid pointer passes
+    # isNil() and reaches the lock dereference, causing a crash.
+    # Expected (after fix): sendRequestToFFIThread returns isErr().
+    # Actual (currently)  : SIGSEGV when the garbage pointer is dereferenced.
+    let invalidCtx = cast[ptr FFIContext[TestLib]](123)
+    let req = ValidationTestRequest.ffiNewReq(dummyCallback, nil)
+    let res = sendRequestToFFIThread(invalidCtx, req)
+    check res.isErr()