devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 6 Actions Packages Projects 1 Releases Wiki Activity

[Security] Harden untrusted-input handling — remote-DoS guards, memory safety, injection defenses #148

New Issue
Open
opened 2026-07-03 10:28:45 +00:00 by jabber.developer2 · 0 comments
jabber.developer2 commented 2026-07-03 10:28:45 +00:00
Collaborator
Copy Link

Hardening of untrusted-input handling: a remote NULL-deref DoS guard, removal of banned string/allocation APIs on peer-controlled data, and input validation / injection defenses (JID, MAM, URL scheme, notifier, control-char/bidi).

Part of #144. Decision legend: M must-do, C could-do, W won't-do (this cycle); investigate = verify the problem is real / scope it before implementing.

Subtasks & findings

T02 — Remote NULL-deref DoS: guard jid_create(from) on receive paths

Owner: Either · Decision: M · Effort: ~0.5d

Confirmed remote crash; copy sibling guard + ASan test.

  • REQ-INP-01 (high · m) — Remote NULL-deref DoS: unguarded jid_create(from) on four receive-path handlers (merges REQ-INP-09)
    • Evidence: message.c:1749-1750; presence.c:397-402,453-454,468-470
    • Fix: Add if(!from_jid) return; at each site (copy sibling _subscribe_handler pattern); add ASan regression test with absent/a@@b from
    • Maps to: NIST PR.PS-06/PR.DS-02; ISO A.8.28/A.8.26

T10 — Memory safety in parsers/crypto (banned APIs, overflow-checked alloc)

Owner: Either · Decision: M · Effort: ~1–1.5d

Shared ox.c g_strdup_printf rewrite; overflow-checked allocations + NULL checks.

  • REQ-MEM-03 (medium · m) — Forbidden strcpy/strcat/sprintf on peer barejids and key/nonce
    • Evidence: ox.c:153-156; omemo/crypto.c:479,483
    • Fix: g_strdup_printf on barejid, snprintf with remaining-size on key/nonce; add CI grep on strcpy/strcat/sprintf in stanza/crypto files (ox.c rewrite shared with REQ-MEM-09)
    • Maps to: NIST PR.PS-01; ISO A.8.28
  • REQ-MEM-04 (high · m) — No overflow-checked allocation; unchecked size*nmemb/realloc and NULL-unchecked mallocs
    • Evidence: omemo.c:1440,455/475/492; http_upload.c:101-103
    • Fix: Use __builtin_mul_overflow/g_malloc_n, NULL-check, save realloc to temp before overwrite; optional UBSan overflow test
    • Maps to: NIST PR.PS-01; ISO A.8.28
  • REQ-MEM-09 (medium · m) — alloca sized from peer barejid length; no -Wvla
    • Evidence: ox.c:150-151
    • Fix: Replace alloca with g_strdup_printf/heap+auto_* (same rewrite as REQ-MEM-03); add -Werror=vla to both build systems
    • Maps to: NIST PR.PS-01; ISO A.8.28/A.8.27

T11 — Input validation & injection (JID tests, MAM gate, URL/scheme, notifier, bidi)

Owner: Either · Decision: M · Effort: ~3–4d · Investigate: investigate INP-05 (disco-gate) and INP-08 sanitizer scope

INP-08 control-char/bidi sanitizer dominates effort.

  • REQ-INP-02 (medium · m) — JID validation lacks prescribed boundary/forbidden/@@/UTF-8 test table
    • Evidence: tests/unittests/xmpp/test_jid.c
    • Fix: Add table-driven suite: 1023/1024 per-part, 3071/3072 total, RFC6122 forbidden chars, double-@, empty parts, invalid UTF-8 (test-only)
    • Maps to: NIST PR.PS-06/PR.DS-02; ISO A.8.28/A.8.26
  • REQ-INP-05 (medium · m (investigate)) — MAM id not disco-gated (XEP-0359 trust)
    • Evidence: message.c:1536-1571 (stored :1409-1410)
    • Fix: Call existing _stanza_id_by_trusted() on the MAM result path; skip strdup when untrusted; toggle-feature unit test
    • Maps to: NIST PR.DS-02/PR.AA-03; ISO A.8.26/A.8.28
  • REQ-INP-06 (medium · m) — /url open|save accept any scheme
    • Evidence: cmd_funcs.c:9633-9655,9678-9698
    • Fix: Whitelist {http,https,aesgcm} via g_uri_parse_scheme after the NULL check; reject file:/javascript:/data: with a unit test
    • Maps to: NIST PR.PS-06/PR.PS-05/PR.DS-02; ISO A.8.28/A.8.26; CIS 9.3
  • REQ-INP-07 (medium · m) — OSX notifier builds shell string and runs via system()
    • Evidence: ui/notifier.c:160
    • Fix: Use g_spawn argv API (or g_shell_quote every interpolated value); quote/$()/backtick injection test
    • Maps to: NIST PR.PS-05; ISO A.8.26/A.8.28
  • REQ-INP-08 (medium · m (investigate)) — No control-char/bidi neutralization or length cap on display/log/DB sinks
    • Evidence: ui/window.c:2024,2120; str_xml_sanitize skips U+202E
    • Fix: Add one control-char/bidi sanitizer + length cap threaded through display, log and DB-write boundaries; ESC/NUL/U+202E unit tests
    • Maps to: NIST PR.DS-01/PR.DS-02/PR.PS-06; ISO A.8.28/A.8.26

Checklist

T02 — Remote NULL-deref DoS: guard jid_create(from) on receive paths

  • REQ-INP-01 · high · M — Remote NULL-deref DoS: unguarded jid_create(from) on four receive-path handlers (merges REQ-INP-09)

T10 — Memory safety in parsers/crypto (banned APIs, overflow-checked alloc)

  • REQ-MEM-03 · medium · M — Forbidden strcpy/strcat/sprintf on peer barejids and key/nonce
  • REQ-MEM-04 · high · M — No overflow-checked allocation; unchecked size*nmemb/realloc and NULL-unchecked mallocs
  • REQ-MEM-09 · medium · M — alloca sized from peer barejid length; no -Wvla

T11 — Input validation & injection (JID tests, MAM gate, URL/scheme, notifier, bidi)

  • REQ-INP-02 · medium · M — JID validation lacks prescribed boundary/forbidden/@@/UTF-8 test table
  • REQ-INP-05 · medium · M · investigate — MAM id not disco-gated (XEP-0359 trust)
  • REQ-INP-06 · medium · M — /url open|save accept any scheme
  • REQ-INP-07 · medium · M — OSX notifier builds shell string and runs via system()
  • REQ-INP-08 · medium · M · investigate — No control-char/bidi neutralization or length cap on display/log/DB sinks

Notes

  • Investigate first (verify/scope before implementing): REQ-INP-05, REQ-INP-08.
Hardening of untrusted-input handling: a remote NULL-deref DoS guard, removal of banned string/allocation APIs on peer-controlled data, and input validation / injection defenses (JID, MAM, URL scheme, notifier, control-char/bidi). Part of #144. Decision legend: **M** must-do, **C** could-do, **W** won't-do (this cycle); *investigate* = verify the problem is real / scope it before implementing. ## Subtasks & findings ### T02 — Remote NULL-deref DoS: guard jid_create(from) on receive paths _Owner:_ Either · _Decision:_ M · _Effort:_ ~0.5d Confirmed remote crash; copy sibling guard + ASan test. - **REQ-INP-01** (high · _m_) — Remote NULL-deref DoS: unguarded jid_create(from) on four receive-path handlers (merges REQ-INP-09) - Evidence: `message.c:1749-1750; presence.c:397-402,453-454,468-470` - Fix: Add if(!from_jid) return; at each site (copy sibling _subscribe_handler pattern); add ASan regression test with absent/a@@b from - Maps to: NIST PR.PS-06/PR.DS-02; ISO A.8.28/A.8.26 ### T10 — Memory safety in parsers/crypto (banned APIs, overflow-checked alloc) _Owner:_ Either · _Decision:_ M · _Effort:_ ~1–1.5d Shared ox.c g_strdup_printf rewrite; overflow-checked allocations + NULL checks. - **REQ-MEM-03** (medium · _m_) — Forbidden strcpy/strcat/sprintf on peer barejids and key/nonce - Evidence: `ox.c:153-156; omemo/crypto.c:479,483` - Fix: g_strdup_printf on barejid, snprintf with remaining-size on key/nonce; add CI grep on strcpy/strcat/sprintf in stanza/crypto files (ox.c rewrite shared with REQ-MEM-09) - Maps to: NIST PR.PS-01; ISO A.8.28 - **REQ-MEM-04** (high · _m_) — No overflow-checked allocation; unchecked size*nmemb/realloc and NULL-unchecked mallocs - Evidence: `omemo.c:1440,455/475/492; http_upload.c:101-103` - Fix: Use __builtin_mul_overflow/g_malloc_n, NULL-check, save realloc to temp before overwrite; optional UBSan overflow test - Maps to: NIST PR.PS-01; ISO A.8.28 - **REQ-MEM-09** (medium · _m_) — alloca sized from peer barejid length; no -Wvla - Evidence: `ox.c:150-151` - Fix: Replace alloca with g_strdup_printf/heap+auto_* (same rewrite as REQ-MEM-03); add -Werror=vla to both build systems - Maps to: NIST PR.PS-01; ISO A.8.28/A.8.27 ### T11 — Input validation & injection (JID tests, MAM gate, URL/scheme, notifier, bidi) _Owner:_ Either · _Decision:_ M · _Effort:_ ~3–4d · _Investigate:_ investigate INP-05 (disco-gate) and INP-08 sanitizer scope INP-08 control-char/bidi sanitizer dominates effort. - **REQ-INP-02** (medium · _m_) — JID validation lacks prescribed boundary/forbidden/@@/UTF-8 test table - Evidence: `tests/unittests/xmpp/test_jid.c` - Fix: Add table-driven suite: 1023/1024 per-part, 3071/3072 total, RFC6122 forbidden chars, double-@, empty parts, invalid UTF-8 (test-only) - Maps to: NIST PR.PS-06/PR.DS-02; ISO A.8.28/A.8.26 - **REQ-INP-05** (medium · _m (investigate)_) — MAM <result> id not disco-gated (XEP-0359 trust) - Evidence: `message.c:1536-1571 (stored :1409-1410)` - Fix: Call existing _stanza_id_by_trusted() on the MAM result path; skip strdup when untrusted; toggle-feature unit test - Maps to: NIST PR.DS-02/PR.AA-03; ISO A.8.26/A.8.28 - **REQ-INP-06** (medium · _m_) — /url open|save accept any scheme - Evidence: `cmd_funcs.c:9633-9655,9678-9698` - Fix: Whitelist {http,https,aesgcm} via g_uri_parse_scheme after the NULL check; reject file:/javascript:/data: with a unit test - Maps to: NIST PR.PS-06/PR.PS-05/PR.DS-02; ISO A.8.28/A.8.26; CIS 9.3 - **REQ-INP-07** (medium · _m_) — OSX notifier builds shell string and runs via system() - Evidence: `ui/notifier.c:160` - Fix: Use g_spawn argv API (or g_shell_quote every interpolated value); quote/$()/backtick injection test - Maps to: NIST PR.PS-05; ISO A.8.26/A.8.28 - **REQ-INP-08** (medium · _m (investigate)_) — No control-char/bidi neutralization or length cap on display/log/DB sinks - Evidence: `ui/window.c:2024,2120; str_xml_sanitize skips U+202E` - Fix: Add one control-char/bidi sanitizer + length cap threaded through display, log and DB-write boundaries; ESC/NUL/U+202E unit tests - Maps to: NIST PR.DS-01/PR.DS-02/PR.PS-06; ISO A.8.28/A.8.26 ## Checklist **T02 — Remote NULL-deref DoS: guard jid_create(from) on receive paths** - [ ] **REQ-INP-01** · high · M — Remote NULL-deref DoS: unguarded jid_create(from) on four receive-path handlers (merges REQ-INP-09) **T10 — Memory safety in parsers/crypto (banned APIs, overflow-checked alloc)** - [ ] **REQ-MEM-03** · medium · M — Forbidden strcpy/strcat/sprintf on peer barejids and key/nonce - [ ] **REQ-MEM-04** · high · M — No overflow-checked allocation; unchecked size*nmemb/realloc and NULL-unchecked mallocs - [ ] **REQ-MEM-09** · medium · M — alloca sized from peer barejid length; no -Wvla **T11 — Input validation & injection (JID tests, MAM gate, URL/scheme, notifier, bidi)** - [ ] **REQ-INP-02** · medium · M — JID validation lacks prescribed boundary/forbidden/@@/UTF-8 test table - [ ] **REQ-INP-05** · medium · M · investigate — MAM <result> id not disco-gated (XEP-0359 trust) - [ ] **REQ-INP-06** · medium · M — /url open|save accept any scheme - [ ] **REQ-INP-07** · medium · M — OSX notifier builds shell string and runs via system() - [ ] **REQ-INP-08** · medium · M · investigate — No control-char/bidi neutralization or length cap on display/log/DB sinks ## Notes - **Investigate first** (verify/scope before implementing): REQ-INP-05, REQ-INP-08.
jabber.developer added the
Kind/Security
Priority
Medium
3
 labels 2026-07-03 10:32:13 +00:00
jabber.developer added this to the Plans (2025-2026) project 2026-07-03 10:32:19 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Kind/Security
Priority
Medium
3
Milestone
No items
No Milestone
Projects
Clear projects
No project Plans (2025-2026)
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#148
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?