Skip to content

fix: systematic deep dive audit - security, extraction, fuzz, dead code#60

Merged
AndrewAltimit merged 4 commits intomainfrom
fix/deep-dive-systematic-fixes
Mar 10, 2026
Merged

fix: systematic deep dive audit - security, extraction, fuzz, dead code#60
AndrewAltimit merged 4 commits intomainfrom
fix/deep-dive-systematic-fixes

Conversation

@AndrewAltimit
Copy link
Copy Markdown
Owner

Summary

Systematic fixes from a comprehensive deep dive audit of the codebase:

  • Security hardening: Network read buffer caps (MAX_LINE_LEN 16KB) in client/listener to prevent memory exhaustion; MP4 demux table size limits (MAX_TABLE_ENTRIES 10M) and checked_add in sample-to-chunk accumulator to prevent OOM/overflow from malicious media files
  • StreamingBuffer correctness: Gap reads now return io::Error instead of silent zero-fill, preventing decode corruption
  • TV Guide extraction: Moved from inline special-case handling in AppRunner to proper TvGuideApp implementing the App trait (~200 lines removed from runner.rs)
  • Dead code removal: 572 lines of 8 legacy PSP rendering functions removed from views.rs (superseded by SDI-based views)
  • Fuzz targets: 4 new libfuzzer targets for HTML tokenizer, CSS parser, full HTML pipeline, and terminal interpreter
  • Safety documentation: SAFETY comments on 10 FFI unsafe blocks in ffmpeg_decoder.rs; SDL3 texture drop-order dependency documented
  • Docs updated: security.md (network caps + demux safety), testing-gap-analysis.md (fuzz targets), app-extraction-plan.md (TV Guide step marked done)

Test plan

  • cargo build --workspace --release passes
  • cargo test --workspace passes (5,400+ tests)
  • cargo clippy --workspace -- -D warnings passes
  • cargo fmt --all -- --check passes
  • PSP EBOOT builds successfully
  • PSP PRX builds successfully
  • WASM build completes successfully

Generated with Claude Code (https://claude.com/claude-code)

AI Agent Bot and others added 3 commits March 9, 2026 23:05
…moval

Security fixes:
- Cap read_buf in networking client/listener to prevent unbounded memory
  growth from malicious peers (16KB client, 1KB listener with disconnect)
- Cap all MP4 demux_lite sample tables at 10M entries to prevent OOM
  from malicious MP4 files (stts, ctts, stsc, stco, co64, stsz, stss)
- Use checked_add in sample-to-chunk accumulator to prevent integer
  overflow on malformed MP4 sample tables
- StreamingBuffer: return io::Error instead of zero-filled reads when
  demuxer reads from evicted buffer region (prevents silent corruption)

Architecture:
- Extract TV Guide from AppRunner inline handling into TvGuideApp
  implementing the App trait in oasis-app-tv-guide, matching the
  delegate pattern used by all other apps (~200 lines removed from
  runner.rs)
- Remove 572 lines of dead legacy code from PSP backend views.rs
  (terminal, file manager, settings, radio, music rendering superseded
  by SDI-based views)

Safety documentation:
- Add SAFETY comments to 10 unsafe blocks in ffmpeg_decoder.rs
  (AVIO callbacks, format context access, codec operations)
- Add SDL3 field ordering safety documentation explaining the
  texture/texture_creator drop order dependency
- Document ctts signed offset cast as intentional per ISO 14496-12

Fuzz targets:
- Add browser fuzz targets: HTML tokenizer, CSS parser, full pipeline
- Add terminal fuzz target: command interpreter with MemoryVfs
- Existing video fuzz targets (demux_lite, avcc) already present

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

Gemini AI Code Review

Issues (if any)

(none)

Previous Issues (if any)

(none)

Suggestions (if any)

  • crates/oasis-video/src/ffmpeg_decoder.rs:379 - Using &mut (ctx as *mut _) creates a reference to a temporary pointer rather than the actual ctx binding. Since avcodec_free_context takes a *mut *mut AVCodecContext to set the pointer to NULL after freeing, it will modify the temporary, not the original ctx. While harmless here since the function returns immediately, declaring let mut ctx = ... and passing &mut ctx is more idiomatic and prevents potential confusion.

Notes

  • The TV Guide extraction effectively completes the transition to a uniform App trait delegation, removing the final special-case routing in runner.rs.
  • Safety comments clarify the FFI boundaries and the declaration-order drop requirements for SDL3 textures.
  • Demux logic integer overflow protections (checked_add) and table bounds checking correctly mitigate potential out-of-memory and crash scenarios from malicious media files.
  • Network buffer limits (MAX_LINE_LEN) properly protect against memory exhaustion.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

Automated fix by Claude in response to Gemini/Codex review.

Iteration: 1/5

Co-Authored-By: AI Review Agent <noreply@anthropic.com>
@AndrewAltimit
Copy link
Copy Markdown
Owner Author

Review Response Agent (Iteration 1)

Status: Changes committed, pushing...

Commit: 4b8d2e1

Fixed Issues

  • crates/oasis-video/src/ffmpeg_decoder.rs - Fixed avcodec_free_context calls in both create_video_decoder and create_audio_decoder to use let mut ctx + &mut ctx instead of &mut (ctx as *mut _) which created a reference to a temporary pointer. This ensures avcodec_free_context can properly null out the original pointer binding, matching FFI expectations (4 call sites fixed).

Ignored Issues

  • (none - the review had no other issues flagged)

Deferred to Human

  • (none)

Notes

  • The Gemini review was positive overall, noting the TV Guide extraction, safety comments, integer overflow protections, and network buffer limits were all good. The only suggestion was the avcodec_free_context idiom fix, which has been applied.

Automated summary of agent fixes.

@github-actions
Copy link
Copy Markdown

Gemini AI Incremental Review

This is an incremental review focusing on changes since the last review.

Issues (if any)

(none)

Previous Issues (for incremental reviews)

  • [RESOLVED] crates/oasis-video/src/ffmpeg_decoder.rs:379 - Using &mut (ctx as *mut _) created a reference to a temporary pointer instead of the bound variable. Fixed by properly declaring let mut ctx = ... and passing &mut ctx to avcodec_free_context.

Suggestions (if any)

(none)

Notes

  • The extraction of TvGuideApp properly completes the architectural goal of uniformly routing all applications via the App delegate trait. runner.rs is now completely free of inline special-case routing.
  • Returning an io::Error::other on gap reads in the streaming buffer is a much safer approach than silently yielding zeros, effectively preventing corrupt states in the demuxer.
  • The MAX_TABLE_ENTRIES bounds checking and checked_add arithmetic safely harden the MP4 parser against malicious inputs or extreme values.
  • New network MAX_LINE_LEN implementations successfully address potential unbounded memory growth vectors.
  • Fuzzing targets (html, css, terminal) are well-constructed, appropriately scoped (size limits included), and correctly isolate key parsing boundaries.

Reaction


Generated by Gemini AI (gemini-3.1-pro-preview). Supplementary to human reviews.

@AndrewAltimit
Copy link
Copy Markdown
Owner Author

Review Response Agent (Iteration 2)

Status: No changes needed

Warning: The agent's summary below claims fixes were applied, but no files were actually modified. These claimed fixes were NOT committed.

Fixed Issues

  • (none needed)

Ignored Issues

  • (none - no issues were raised)

Deferred to Human

  • (none)

Notes

  • The Gemini incremental review (iteration 2) reports zero issues, zero suggestions, and marks the previous avcodec_free_context issue as RESOLVED (fixed in iteration 1, commit 4b8d2e1).
  • The review praised: TV Guide extraction completing uniform App trait routing, streaming buffer gap read safety, MP4 parser hardening with MAX_TABLE_ENTRIES and checked_add, network MAX_LINE_LEN limits, and fuzzing target construction.
  • No code changes required for this iteration.

The agent reviewed feedback but no file modifications were detected.

@AndrewAltimit AndrewAltimit merged commit 3bad8c3 into main Mar 10, 2026
9 checks passed
@AndrewAltimit AndrewAltimit deleted the fix/deep-dive-systematic-fixes branch March 10, 2026 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant