description: "Safety audit — unsafe blocks, memory leaks, undefined behavior, Vulkan spec violations"
Safety Audit
Read _audit-common.md (layout, methodology, dedup, report format) and
_audit-severity.md (the unified scale + the Special-Rules table this domain
leans on heavily) before starting. Do not restate their content here.
Severity anchors for this domain (from _audit-severity.md):
FFI lifetime violation = CRITICAL · BLAS/TLAS wrong geometry/address or
SSBO index mismatch = CRITICAL · leak that compounds per frame = HIGH ·
Vulkan spec violation = HIGH · unsafe without a safety comment = MEDIUM.
Scale of the surface
unsafe is concentrated, not scattered: ~760 occurrences live in
crates/renderer/src (ash FFI + gpu-allocator), then a long tail —
~11 each in crates/nif and crates/fsr3-sys (the vendored FSR 3.1 FFI,
Dimension 1), ~6 in crates/core, 2 in byroredux, and one each in
crates/plugin, crates/facegen, crates/cxx-bridge, crates/ui, and
crates/pex (the M47.2 decompiler — its single unsafe is a guarded
transmute in opcode.rs, see Dimension 2). crates/save (M45) and
crates/hkx (M47.2, a deliberately safe packfile reader) have no unsafe.
Counts drift — recount with grep -ro unsafe crates/<c>/src | wc -l rather
than trusting these figures. Renderer carries roughly nine SAFETY comments
per ten unsafe tokens; the residual gap is where the unsafe-without-comment
(MEDIUM) findings live. Budget your time accordingly — do not audit the
nif/core/pex tail at the expense of the renderer FFI mass. The Dimension-4
sweep greps all of crates/ so pex / save / fsr3-sys / hkx are covered
automatically.
Dimensions below are ordered by safety blast radius: FFI lifetime, then memory-corruption/UB, then per-frame leaks, then unsafe-block discipline, then Vulkan-spec compliance, then the narrower regression-guard surfaces.
Dimensions
1. FFI Lifetime Safety (cxx bridge) — CRITICAL class
- The cxx surface is currently a placeholder.
crates/cxx-bridge/src/lib.rsexposes one bridge fn,native_hello() -> String(impl incrates/cxx-bridge/cpp/native_utils.cpp). There is no raw-pointer exchange, no Rust-string-into-C++ borrow, no shared-ownership handoff across the boundary today. Do NOT report speculative "string lifetime / dangling pointer across cxx" findings against this crate — they describe a surface that does not exist yet. The real check here is a scope guard: confirm the bridge still has no owned-pointer / borrowed-slice signatures. The instant a*const,&[u8],Box<…>, orunsafe extern "C++"fn taking a Rust reference appears, this becomes a live CRITICAL-class dimension and the lifetime analysis from_audit-severityapplies. unsafe extern "C++"in the bridge marks the C++ side as trusted — verify no new fn returns a pointer Rust then dereferences past the call.crates/fsr3-sys(added 2026-07-22) is a real, live FFI crossing — unlike the cxx-bridge placeholder above, this is not hypothetical.extern "C"functions take*mut RawContext/*const RawCreateDesc/*mut RawVersionetc.;pub unsafe fn Context::create/Context::dispatchcarry# Safetydoc sections stating caller contracts (device/physical-device/proc-addr must outlive theContext; dispatch handles must belong to the creating device), andDropcalls back into the native shim. Audit everyunsafe fnhere for a# Safetydoc and a lifetime contract the way this dimension used to reserve for a hypothetical live cxx-bridge.
2. Memory Corruption / UB
- ECS cached-pointer contract (regression guard, #35 + #1367).
World::get(crates/core/src/ecs/world.rs) returns aComponentRef<'_, T>, NOT a raw pointer with a dropped guard (the unsound #35 pattern).ComponentRef,StorageRef, andStorageRefMutincrates/core/src/ecs/query.rscache a*const T/*mut Tresolved once innew()and deref it in the hot path (#1367). Each cached-derefunsafeblock carries a SAFETY comment tying the pointer's validity to the lock guard the wrapper pins. The invariant: the guard must outlive every deref, and&mut selfmust gate&mut *self.storage. Verify the SAFETY comments still match the field layout and that no refactor let a guard drop before its pointer (use-after-free → CRITICAL). #[repr(C)]GPU-struct soundness (crates/renderer/src/vulkan/scene_buffer/gpu_types.rs):GpuInstance/GpuCamera/GpuLightetc. are uploaded byte-for-byte to SSBOs. vec3 must be three scalarf32, never[f32; 3](std430 vec3 padding). A layout drift here is silent per-instance corruption — see Dimension 6 for the GpuMaterial pin and_audit-severity's#[repr(C)]-drift HIGH row.- NIF bulk POD reads (
NifStream::read_pod_vec,crates/nif/src/stream.rs; the header mirrorread_pod_vec_from_cursor,crates/nif/src/header.rs):read_exactof raw LE bytes into aT: AnyBitPatternvector. SAFETY comments must hold —Tis restricted to bit-pattern-safe types (a sealed bound stopsread_pod_vec::<bool>). Verify the byte-count overflow guard (count × size) is present and no caller widensTpastAnyBitPattern. - sfmaterial enum decode (
BuiltinType::from_u32,crates/sfmaterial/src/types.rs): MUST stay a checkedmatchover the0xFFFFFF##tags with a_ => return Err(Error::UnsupportedBuiltin { raw })arm (confirmed present). The module doc's "transmute into this enum" wording is aspirational prose, NOT the impl — an actualstd::mem::transmuteof an unmatched#[repr(u32)]byte pattern is UB. Verify thematch+Errarm survive any "optimization." - pex opcode decode (
OpCode::from_u8,crates/pex/src/opcode.rs): unlike sfmaterial, this one IS a realunsafe { std::mem::transmute::<u8, OpCode>(byte) }— sound ONLY because the SAFETY comment's two preconditions hold:OpCodeis#[repr(u8)]with contiguous discriminants0..MAX_OPCODE, ANDbyteis range-checked (< MAX_OPCODE) before the transmute. Both must stay true: a gap in the discriminant sequence, or a refactor that drops the bound check, makes an out-of-range byte UB. Verify the guard and the contiguity (no skipped values in the enum) on any opcode-table change. - Stack-overflow risk: no unbounded recursion in block-walk / scene-graph traversal.
3. Memory & Resource Leaks (HIGH when per-frame/per-cell)
- Rapier bodies on cell unload (regression guard, #1520,
34c7a218).crates/physics/src/world.rs::remove_*andbyroredux/src/cell_loader/unload.rsmust release a cell's rigid bodies, colliders, and impulse joints fromRigidBodySet/ColliderSet/ImpulseJointSet(plus broad-phase / query-pipeline state) when the cell unloads. Without it they accumulate per cell — a steady leak under exterior streaming. Guard test:byroredux/src/cell_loader/rapier_release_tests.rs. Verify the release path is still wired and the test still asserts emptiness post-unload. - Deferred-destroy drain (
crates/renderer/src/deferred_destroy.rs,DeferredDestroyQueue<T>shared by mesh + BLAS + BLAS-scratch buffer (#1782) + texture + skin compute): objects are destroyed only after the in-flight fence clears (#418 moved the tick after fence wait; #732 added an explicit shutdown drain). Verify the tick still runs after fence wait incontext/draw.rsand the shutdown sweep drains the queue — a missed drain leaks GPU memory across the app lifetime, a too-early destroy is use-after-free (CRITICAL). AllocatorResourcedrop ordering (regression guard, #1406,299e6a84).AllocatorResource(crates/renderer/src/vulkan/allocator.rs; held inbyroredux/src/main.rs) must be removed from the ECSWorldBEFOREVulkanContext::drop()runs. The allocator holds a liveArc<Device>; if theWorldoutlives the context, the allocator'sDropcalls the driver against a destroyed logical device (use-after-free → CRITICAL). Verify the main loop removes the resource before dropping the renderer, including the panic-unwind path that could skip the removal.- GPU allocation inventory — every long-lived allocation tracked and freed:
BLAS scratch/result, TLAS instance/result, G-buffer images, SVGF history, TAA
per-FIF history images, caustic + water-caustic R32_UINT accumulators
(
caustic.rs/water_caustic.rs), per-skinned-entity SkinSlot output buffers, MaterialBuffer SSBO, volumetric/bloom mip pyramids. Cross-check eviction thresholds againstdocs/engine/memory-budget.md; do not re-derive. - CPU-side unbounded growth —
Vec/HashMapkeyed by cell or path that never shrinks. The MaterialTable dedup map and AnimationClipRegistry (Dimension 8) are the known per-cell-growth risks.
4. Unsafe-Block Discipline (MEDIUM — the bread-and-butter sweep)
- Grep every
unsafeincrates/+byroredux/(.rs). For each: is there a SAFETY comment, and does the comment's stated invariant actually hold at this call site? A correct unsafe block with no comment is still a MEDIUM finding (_audit-severitySpecial Rules). A commented block whose invariant is FALSE is the higher-severity finding. - Heaviest in
crates/renderer/src/vulkan/ash FFI — the SAFETY/unsafe count gap (~676 vs ~761) is the haystack. Spot-check the ash dispatch wrappers, the gpu-allocatorArc<Mutex<…>>interactions, and anyfrom_raw_parts/caston mapped memory. - Report unsafe blocks lacking comments as a batched MEDIUM finding (list the sites) rather than one finding per block, unless an invariant is actually unsound.
5. Vulkan Spec Compliance (HIGH — but flag what cargo test can't see)
Per the No-Speculative-Vulkan-Fixes rule: render-pass / barrier / pipeline-state spec claims that are invisible to
cargo testMUST be framed as "needs validation-layer or RenderDoc verification", not asserted as confirmed bugs. Run the engine with validation layers (debug build) and report ANY emitted error verbatim — that is the sound evidence channel for this dimension.
- All
vkCreate*/vkDestroy*paired; Drop ordering destroys children before parents (device-destroy is last). - Queue submission ordering: wait-before-signal; per-image semaphores.
- Acceleration structures (
crates/renderer/src/vulkan/acceleration/): correct geometry flags, valid device addresses, buffers carrySHADER_DEVICE_ADDRESS. TLAS UPDATE mode — instance/geometry count must match the original BUILD. Skin BLAS refit — vertex/geometry count must match BUILD; a bone-count change forces a full rebuild. (Wrong AS geometry/address = CRITICAL per_audit-severity.) - TLAS resize wait (regression guard, #1390,
a7e1502b). The resize branch inacceleration/tlas.rscallsdevice.device_wait_idle()before freeing the old allocation (confirmed present). Verify the wait survives — without it the GPU may still consume the old TLAS scratch during free under a resize-under-load refactor. VK_KHR_ray_queryenabled + feature-gated before any ray-query use.- Per-frame compute layout hygiene (TAA / caustic / water-caustic / volumetrics /
bloom): images that coexist as storage-write + sampled-read are held in
GENERAL;initialize_layoutsdoes the one-time UNDEFINED→GENERAL transition for every mip / FIF slot. A missed slot is an UNDEFINED-read validation error. CLEAR-before- COMPUTE invariant (caustic R32_UINTimageAtomicAdd, volumetric inject) — a missing clear is persistent cross-frame ghost accumulation. Verify the volumetrics caller honors the dispatch gate:VOLUMETRIC_OUTPUT_CONSUMED(crates/renderer/src/vulkan/volumetrics.rs) is nowtrue, so the pass is live — dispatch is dead only while it readsfalse. Callers MUST gatevol.dispatch()on that const either way (context/post_passes.rs); read the const rather than assuming a state. - SPIR-V reflection (
crates/renderer/src/vulkan/reflect.rs): the Rust descriptor layout must match shader-declared bindings — this is the one binding-drift check that IS visible tocargo test(scene_descriptor_reflection_tests). Prefer it over eyeballing descriptor writes.
6. R1 Material Table Layout Soundness
GpuMaterialsize is pinned at 348 B bygpu_material_size_is_348_bytes(crates/renderer/src/vulkan/material.rs) — the test name now matches the asserted size (history: 272 → 260 after #804 droppedavg_albedo, → 296 with the Disney sheen/subsurface lobe #1249, → 300 withanisotropic#1250, → 348 on 2026-07-27 (1d94eb24) with the twelve common supplemental texture roles). A stale 260/272/296/300 in audit prose, or any test-name-vs-asserted-size mismatch, means the GPU is reading wrong bytes.- Per-field offset pin
gpu_material_field_offsets_match_shader_contract(#806): every named field's byte offset asserted against the shader contract. The size pin alone cannot catch a within-vec4 reorder (swaptexture_index ↔ normal_map_indexis size-invisible, runtime-lethal). Adding a field without updating this assertion is a regression. - ALL fields are flat scalar
f32/u32— never[f32; 3](std430 vec3 alignment). This includes the newest scalars: the BGSM translucency suite (translucency_subsurface_r/g/b,…_transmissive_scale,…_turbulence) and the Disney lobe (ior,subsurface,sheen,sheen_tint,anisotropic). - Pad fields explicitly zeroed (the byte-
Hash/Eqdedup hashes the raw 348 B; an uninit hole poisons dedup). New scalars must be zeroed inGpuMaterial::default()so default materials still dedup to slot 0. - Intern cap (#797).
MaterialTable::interncaps atMAX_MATERIALS = 16384(scene_buffer/constants.rs); over-cap interns return id0with a one-shot warn — no SSBO over-index, no DEVICE_LOST.upload_materials(scene_buffer/upload.rs)debug_assertslen <= MAX_MATERIALSand clamps with.min(MAX_MATERIALS). Verify the intern cap and the upload truncation stay in lockstep. GpuInstance.material_idindexes the SSBO with NO GPU bounds check — CPU must guarantee in-range (SSBO index mismatch = CRITICAL).ui.vertMaterialBuffer read offsets must stay in lockstep with the canonicalstruct GpuMaterial/GpuInstanceincrates/renderer/shaders/include/bindings.glsl(triangle.frag#includes it) — #785 was a stale-hunk regression reading wrong bytes — nameui.vertexplicitly.
7. RT IOR-Refraction Safety (regression guards)
- Glass-passthrough loop guard (#789): the texture-equality identity check at the refraction hit prevents unbounded recursion when coincident glass surfaces share an albedo/normal-map descriptor pair. A regression is a frame-time hang on any paired-glass cell. Verify the check is present.
- Glass ray budget
GLASS_RAY_BUDGET(crates/renderer/src/shader_constants_data.rs, mirrored incrates/renderer/shaders/include/shader_constants.glsl— verify the two stay in lockstep; raised from 8192 in6efe1706, and again since — check the constant by name rather than trusting a hard-coded figure here). It is a runaway-recursion cap, not a quality knob. #1438 documented that the atomicAdd accounting can overshoot the budget unconditionally — note that nuance rather than reporting the overshoot as new. Verify the budget is enforced at every glass call site. - Frisvad orthonormal basis (#820): the naive
cross(N, world-up)basis degenerates near-vertical (zero-length → NaN ray). Verify Frisvad is the active path for IOR refraction roughness spread. - IOR miss fallback for interiors uses cell-ambient, not global sky tint (open-sky leakage into dungeons is a visible regression).
DBG_VIZ_GLASS_PASSTHRU = 0x80is a permanent diagnostic bit — verify it hasn't collided with a new debug flag (full catalog incrates/renderer/src/shader_constants_data.rs, mirrored to the generatedcrates/renderer/shaders/include/shader_constants.glsl).
8. NPC / Animation Spawn Safety
- B-spline pose-fallback sentinel (#772): NPCs vanishing under FNV
BSPSysSimpleColorModifierparticle stacks sharing keyframe time-zero with the actor's player must be gated on anFLT_MAXsentinel. Removing the gate is whole-NPC disappearance, not a stuck pose. Verify the sentinel is wired. - AnimationClipRegistry dedup (#790): the registry interns by lowercased path so cell streaming doesn't grow it unboundedly (otherwise one keyframe set leaks per cell load → steady RAM growth). Verify case-insensitive interning is preserved.
- B-splines reach FNV / FO3 too (
feedback_bspline_not_skyrim_only.md) — do NOT rule outNiBSplineCompTransformInterpolatorby game era. - Starfield content is WALKABLE (Cydonia) — SF cells reach the spawn/animation path; don't short-circuit spawn-safety reasoning with "no SF content exercises this."
MAX_TOTAL_BONESoverflow guard must fire — silent truncation past cap was the M29 regression. The slot-exhaustion warn lives onSkinSlotPool(crates/core/src/ecs/resources/skin_slot_pool.rs), one-shot via theoverflow_warnedflag withoverflow_attempt_countcarrying total demand; excess skinned entities fall back to bind pose rather than over-indexing. Guard tests:byroredux/src/render/bone_palette_overflow_tests.rs.
9. NIFAL Boundary — NaN/Inf on the GPU (UB facet only)
See /audit-nifal for correctness-of-mapping; this dimension covers ONLY the
safety facet — NaN/inf scalars reaching the GPU, unbounded allocation.
byroredux/src/material_translate.rs::translate_materialdeliberately seedsf32::NANintoMaterial.metalness/roughness(mesh.metalness_override.unwrap_or(f32::NAN), same for roughness).Material::resolve_pbr(crates/core/src/ecs/components/material.rs) is the ONLY thing that detects (is_nan()) and clamps these sentinels before they reachGpuMaterial. Both fields are now plainf32(noOption), so a producer that skipsresolve_pbr()ships a NaN into the SSBO silently (NaN-on-GPU = UB). Verify EVERY renderer-boundMaterialproducer runsresolve_pbr()or constructs already-finite values (thestatic_meshes.rsfallback constructs finite defaults directly — confirm it still does).- Collision translate (
crates/nif/src/import/collision/mod.rs, coversBhkMultiSphereShape+BhkConvexListShape): emitted half-extents / radii / sphere centers must be finite and bounded — a NaN/inf shape param propagates into the physics solver and the BLAS build. - Typed particle blocks (
crates/nif/src/blocks/particle.rs) →extract_emitter_params/extract_emitter_rate(crates/nif/src/import/walk/mod.rs) →apply_emitter_params(byroredux/src/systems/particle.rs): emitter rate / lifespan / size must be finite and non-negative at the extract boundary — an unbounded or NaN rate is an unbounded-allocation / NaN-transform risk downstream.
10. debug-ui (egui overlay) Teardown & Shared-Allocator Safety
crates/debug-ui/src/lib.rsDebugUiStateis the CPU half only — egui context,egui_winitstate, lastFullOutput, panel state. It holds no Vulkan handle. It lives as an ECS resource (impl Resource for DebugUiState) and is owned by the main loop.- The Vulkan half is
EguiPass(crates/renderer/src/vulkan/egui_pass.rs, held asVulkanContext::egui_pass: Option<EguiPass>). It takes anash::Device+ the sharedArc<Mutex<gpu_allocator::vulkan::Allocator>>and wrapsegui-ash-renderer, which owns its own descriptor pool + per-texture images, plusEguiPass's ownvk::RenderPass+ per-swapchain-image framebuffers. Those MUST be freed before the engine destroys theash::Device— same class as Dimension 3's allocator-before-device rule. Verify theOption<EguiPass>teardown runs ahead ofVulkanContext's device-destroy. - Texture free is deferred one frame (
pending_free), leaning ondraw_frame's fence wait — verify the defer survives; freeing on the arriving frame is a use-after-free. - The allocator mutex is SHARED with the render thread — verify it is held for
minimum duration during egui texture upload; a long hold stalls rendering. The
graphics queue is likewise passed as a
Mutex(EguiDispatchCtx::queue) so the lock scopes to theset_texturessubmit only (CONC-D1-01 / #1713) — a widened hold regresses that.
Procedure
- Grep all
unsafeincrates/+byroredux/(.rs); note the renderer mass and the SAFETY-comment gap (Dimension 4). - Confirm the cxx bridge is still a no-pointer placeholder (Dimension 1).
- Audit the cached-pointer ECS contract + repr(C) GPU structs + NIF POD reads + sfmaterial decode (Dimension 2).
- Walk the leak inventory and the three drop-ordering regression guards — Rapier release, deferred-destroy drain, AllocatorResource removal (Dimension 3).
- Sweep unsafe-block discipline; batch the comment-less blocks (Dimension 4).
- Vulkan-spec pass — run validation layers, report emitted errors verbatim; frame barrier/layout claims invisible to cargo test as "needs RenderDoc verification" (Dimension 5).
- R1 material layout pins (Dimension 6), IOR/glass guards (7), NPC/anim spawn (8), NIFAL NaN boundary (9), debug-ui teardown (10).
- Dedup against open/closed issues (
_audit-commonDeduplication) — most items above are regression guards; recast a confirmed-intact guard as PASS, not a NEW finding. - Save the report to
docs/audits/AUDIT_SAFETY_<TODAY>.md(see_audit-commonReport Finalization).
Auditeur de Securite
Securite
Analyse le code pour detecter les vulnerabilites OWASP Top 10.
Checklist de Sécurité OWASP
Securite
Génère des checklists de sécurité applicative basées sur l'OWASP Top 10.
Modélisation de Menaces
Securite
Génère des documents de modélisation de menaces avec analyse STRIDE.