Skip to content

fix: eight separate calls to strcpy() and strcat() i... in vmsmail.c#187

Merged
k21971 merged 2 commits into
k21971:masterfrom
orbisai0security:fix-vmsmail-buffer-overflow-strcpy-strcat
May 14, 2026
Merged

fix: eight separate calls to strcpy() and strcat() i... in vmsmail.c#187
k21971 merged 2 commits into
k21971:masterfrom
orbisai0security:fix-vmsmail-buffer-overflow-strcpy-strcat

Conversation

@orbisai0security
Copy link
Copy Markdown
Contributor

@orbisai0security orbisai0security commented May 9, 2026

Summary

Replace several unbounded string copies in the OpenVMS mail/broadcast parser with bounded formatting.

Details

sys/vms/vmsmail.c builds display text and response commands from captured VMS mail, phone, talk, and broadcast messages using fixed-size buffers (txt_buf, cmd_buf, and nam_buf). Some paths used strcpy()/strcat() to append message-derived text into those buffers.

Although broadcast input is already bounded, prefixes such as "Mail for you: " or "MSG +" can cause the constructed output to exceed the destination buffer. This patch uses snprintf() consistently so long messages are truncated safely.

The test-driver-only gets() calls are also replaced with fgets().

Security impact

This is defensive memory-safety hardening for VMS-specific mail/broadcast handling. Exploitability depends on VMS build configuration and whether an attacker can deliver crafted broadcast/mail/talk text to a running game process.

Changes

  • Replace unbounded strcpy()/strcat() constructions with snprintf()
  • Replace test-driver gets() calls with fgets()
  • Preserve existing parser behavior while truncating oversized constructed strings safely

Verification

  • Build passes
  • Scanner re-scan confirms fix
  • LLM code review passed

Automated security fix by OrbisAI Security

Automated security fix generated by Orbis Security AI
@entrez
Copy link
Copy Markdown
Collaborator

entrez commented May 9, 2026

The VMS servers with EvilHack could already be pwned... :(

@orbisai0security
Copy link
Copy Markdown
Contributor Author

Fair 😅, I agree the practical exploitability depends heavily on whether anyone is actually running the VMS build and how exposed those servers are.

I’d frame this as defensive hardening: vmsmail.c builds fixed-size strings from message-derived input plus prefixes, so the old strcpy/strcat paths could overflow even when the original broadcast buffer was bounded. This patch just makes those paths truncate safely with snprintf().

I've adjusted the PR description so it doesn’t overstate the impact.

@k21971 k21971 self-assigned this May 13, 2026
@k21971 k21971 added the bug Something isn't working label May 13, 2026
@k21971
Copy link
Copy Markdown
Owner

k21971 commented May 13, 2026

Hey there. Yeah entrez is correct, I've never supported the VMS build, but your PR is valid, and the fixes check out. Having said that, the commit is incomplete. Same treatment could be applied to sys/vms/vmsmail.c:246 and 249-251. Maybe convert those to use snprintf(cmd_buf, sizeof cmd_buf, ...) / snprintf(txt_buf, sizeof txt_buf, ...) to match the rest of your commit. Address those and I'll merge your PR. And maybe actually see about supporting a VMS build hah.

Convert remaining Sprintf calls in JNET send block to snprintf with
sizeof bounds, as requested in PR k21971#187 review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@orbisai0security
Copy link
Copy Markdown
Contributor Author

I've made the changes to use snprintf in 2 other cases you pointed to. Hope this is good to go now.

@k21971 k21971 merged commit 66ab786 into k21971:master May 14, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants