Skip to content

fix: multiple critical bugs and safety improvements - #60

Open
hobostay wants to merge 1 commit into
bytedance:masterfrom
hobostay:fix/critical-bugs-and-safety-improvements
Open

fix: multiple critical bugs and safety improvements#60
hobostay wants to merge 1 commit into
bytedance:masterfrom
hobostay:fix/critical-bugs-and-safety-improvements

Conversation

@hobostay

Copy link
Copy Markdown

Summary

  • Fix critical rm -rf ... /dev/null bug in volcuninstall.sh that would destroy the system null device
  • Fix heap buffer overflow in sbd.file.c caused by malloc(strlen(ptr+1)) (wrong parenthesization)
  • Fix NULL pointer dereference in lim.main.c where fgets() was called after popen() returned NULL
  • Fix duplicate NULL check in bhosts.c (loadval[i] checked twice instead of loadval1[i])
  • Fix buffer overflow in lib.i18n.c by replacing vsprintf with vsnprintf
  • Fix file descriptor leaks in elock.c on 4 error paths
  • Fix memory leak in mbd.requeue.c where realloc failure lost the original pointer
  • Guard against NULL from getenv("USER") in cmd.sub.c
  • Fix memory leak in res.handler.c on XDR decode error path
  • Fix "ByteBance" typo to "ByteDance" and "lim mbatchd" to "mbatchd pid" in README.md

Details

Critical: volcuninstall.sh would destroy /dev/null

Line 111 had /dev/null as an argument to rm -rf:

rm -rf "${VOLC_TOP}/${MIX_OS_FOLDER}" "${VOLC_TOP}/share" "${VOLC_TOP}/include" /dev/null 2>&1 || true

This would delete /dev/null on Linux systems, breaking most programs. Also fixed missing echo command on line 22 and unquoted variables in multiple rm -rf calls.

Critical: Heap buffer overflow in sbd.file.c

malloc(strlen(spooledExecName+1)) computes strlen starting one byte after the beginning, allocating 2 bytes too few. The subsequent strcpy overflows the buffer. Same bug at two locations (lines 1344 and 2236).

High: NULL pointer dereference in lim.main.c

After popen() returns NULL, the code fell through to fgets(fp, ...) causing a crash. Added return -1.

High: File descriptor leak in elock.c

touchElock() opened a lock file but on 4 different error paths (lseek/read/write failures), returned without closing the file descriptor. In a long-running daemon this would exhaust fd limits.

Medium: vsprintf buffer overflow in lib.i18n.c

vsprintf was used with a fixed 1024-byte buffer with no bounds checking. Replaced with vsnprintf.

Test plan

  • Verify volcuninstall.sh no longer includes /dev/null in rm command
  • Verify echo command present on error message line
  • Build the project and verify no new warnings introduced
  • Test bhosts command with various host configurations
  • Test bsub error message display with USER env var unset

🤖 Generated with Claude Code

Fix several bugs found across the codebase including:

1. volcuninstall.sh: Remove /dev/null from rm -rf arguments (would
   destroy the system null device), add missing echo command for
   error message, quote all variables in rm/path operations to
   prevent word splitting issues.

2. sbd.file.c: Fix off-by-one in malloc(strlen(ptr+1)) which should
   be malloc(strlen(ptr)+1). The wrong parenthesization computes
   the length of the string starting one byte later, causing heap
   buffer overflows at two locations.

3. lim.main.c: Add missing return after popen() NULL check to
   prevent NULL pointer dereference in subsequent fgets() call.

4. bhosts.c: Fix duplicate NULL check (loadval[i] checked twice
   instead of checking loadval1[i]).

5. lib.i18n.c: Replace vsprintf with vsnprintf to prevent buffer
   overflow in static 1024-byte i18nPrintBuffer.

6. elock.c: Add close(lock_fd) on four error paths that were
   leaking file descriptors in touchElock().

7. mbd.requeue.c: Fix realloc pattern that loses original pointer
   on allocation failure, causing memory leaks in the daemon.

8. cmd.sub.c: Guard against NULL return from getenv("USER") in
   error message formatting.

9. res.handler.c: Free tempBuf on XDR decode error path that was
   leaking memory.

10. README.md: Fix "ByteBance" typo to "ByteDance", fix "lim
    mbatchd" to "mbatchd pid" in status output example.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@CLAassistant

CLAassistant commented Apr 19, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


chu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@qinan-cn

Copy link
Copy Markdown
Collaborator

Thanks for your contribution and bug fixes! We will review these changes carefully later and get back to you :)

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.

3 participants