Skip to content

Security CI: ASAN/UBSAN + CodeQL#27

Open
pronskiy wants to merge 5 commits intomainfrom
security-ci
Open

Security CI: ASAN/UBSAN + CodeQL#27
pronskiy wants to merge 5 commits intomainfrom
security-ci

Conversation

@pronskiy
Copy link
Copy Markdown
Collaborator

@pronskiy pronskiy commented Mar 22, 2026

What

Add two security-focused CI workflows:

  1. ASAN + UBSAN — Builds PHP from source with AddressSanitizer and UndefinedBehaviorSanitizer, then runs the full test suite. Catches memory bugs (use-after-free, buffer overflow, fd leaks, signed overflow) at runtime.
  2. CodeQL — GitHub semantic analysis with security-extended queries. Catches common C vulnerability patterns statically (every push/PR + weekly).

Trimmed from the original PR #8 — removed Cppcheck and Clang Static Analyzer (overlap with CodeQL, noisy on inherited Xdebug code).

Attack Surface Analysis

This PR is informed by a threat model of the extension. Key vectors:

Network — DBGp (TCP :9003)

Vector Severity Status
Unauthorized debug attach — anyone reaching :9003 gets code exec via eval Critical Mitigated by localhost-only default; IDE connection prompts planned (SPEC §7.2)
Malicious DBGp client — crafted XML to crash extension (buffer overflow, XXE) High Inherited Xdebug XML parser, not yet audited post-fork
Connection flood / DoS — exhaust FDs via connection spam Medium No rate limiting yet
MitM on DBGp — plaintext XML, sniffable on non-localhost Medium Spec recommends localhost-only

Memory Safety (C extension)

Vector Severity Status
Use-after-free — socket FD reuse after close High Fixed in PR #11 (socket = -1 after close)
FD leak — failed init does not close socket Medium Fixed in PR #11
Buffer overflow in DBGp parser — inherited Xdebug XML parsing High This PR adds ASAN + CodeQL to catch these
Stack overflow — deep recursion in variable serialization Medium max_depth INI limits this

Information Disclosure

Vector Severity Status
Variable inspection exposes secrets — eval/inspect reads env vars, passwords Critical By design (dev tool only, SPEC §1)
Stack traces leak paths Low Unavoidable for a debugger

Not Applicable (yet)

  • Signal handler safety — profiler SIGPROF not implemented yet (Phase 4)
  • Trigger injection — auto-connect default makes triggers irrelevant
  • Network sniffing — localhost only in default config

Files

  • .github/workflows/sanitizers.yml — ASAN + UBSAN on PHP 8.3, 8.4
  • .github/workflows/codeql.yml — CodeQL with security-extended queries

Supersedes #8.

@pronskiy
Add security CI workflows
f2dd859 
- CodeQL: semantic analysis with security-extended queries (every push/PR + weekly)
- ASAN + UBSAN: runtime sanitizers, builds PHP from source with sanitizer flags (every push/PR)
- Cppcheck: fast static analysis for leaks, null derefs, fd leaks (every push/PR)
- Clang Static Analyzer: path-sensitive analysis via scan-build (weekly + manual dispatch)
@github-advanced-security
Copy link
Copy Markdown

github-advanced-security bot commented Mar 22, 2026

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

  • The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
  • Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
  • You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

pronskiy added 4 commits March 22, 2026 21:05
@pronskiy
Fix UBSAN: xdebug_fiber_entry_dtor function type mismatch
526bcae 
The hash destructor callback was declared as taking
struct xdebug_fiber_entry * but registered as xdebug_hash_dtor_t
(void (*)(void *)). Calling through the mismatched pointer type
is undefined behavior per C11 §6.5.2.2.

Fix: accept void * and cast inside the function body.
Zero performance impact — identical generated code.
@pronskiy
Fix DBGp test timeouts under ASAN
51381af 
The DBGp test client had hardcoded 3s/5s socket timeouts. Under ASAN
(2-3x slower), PHP takes longer to respond, causing false test failures.

Fix: make timeouts configurable via DBGP_TIMEOUT env var (defaults
unchanged). Set DBGP_TIMEOUT=10 in the sanitizer CI workflow.
@pronskiy
Run ASAN tests single-threaded
6120ecf 
Parallel ASAN-instrumented PHP processes compete for CPU on CI runners,
causing DBGp socket timeouts. Run with -j1 for reliable results.
@pronskiy
ASAN CI: fail only on sanitizer errors, not test mismatches
322ed0b 
The ASAN-instrumented test suite has ~36-45 test output mismatches
(known XFAIL tests + ASAN-induced timing differences). These are
not memory bugs.

The CI now runs all tests, captures output, and fails ONLY when
actual sanitizer errors are detected (AddressSanitizer, LeakSanitizer,
UBSAN runtime errors). Test output mismatches are logged but
do not block the PR.
@pronskiy pronskiy added the ci CI/CD, GitHub Actions, testing infrastructure label Mar 24, 2026
@carlos-granados
Copy link
Copy Markdown
Collaborator

carlos-granados commented Mar 25, 2026

@pronskiy I've never used these two tools so I am not really sure what we are expecting to see here. I tried to see the results of the CodeQL scan but they seemed empty. And the ASAN + UBSAN test run was full of errors. Do you have more info on what we are expected to see or how to interpret the results?

One thing: for the CodeQL scan we should use V4 of the GitHub action, V3 is deprecated as far as I can tell from the run output

And in the ASAN + UBSAN check you need to update the version of the actions/cache GitHub action as V4 is also deprecated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carlos-granados carlos-granados Awaiting requested review from carlos-granados

Assignees

No one assigned

Labels

ci CI/CD, GitHub Actions, testing infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pronskiy @carlos-granados