Skip to content

fix: stack-buffer-overflow (#1550), wrong statvfs block_size (#1547), path traversal (#1516)#1559

Open
Hardikrepo wants to merge 1 commit into
nasa:devfrom
Hardikrepo:fix/multiple-issues
Open

fix: stack-buffer-overflow (#1550), wrong statvfs block_size (#1547), path traversal (#1516)#1559
Hardikrepo wants to merge 1 commit into
nasa:devfrom
Hardikrepo:fix/multiple-issues

Conversation

@Hardikrepo

Copy link
Copy Markdown

Summary

This PR fixes three bugs found in the dev branch:

Fix #1550 — Stack-buffer-overflow in UtAssert_StringBufCompare()

File: ut_assert/src/utassert.c

Root cause: ScrubbedString1[256] and ScrubbedString2[256] are fixed 256-byte stack buffers. When a fixed-length buffer longer than 255 bytes contains no NUL within the requested bound, FormatLen1/FormatLen2 are set directly from the caller-supplied String1Max/String2Max. The subsequent memcpy writes past the end of the stack buffers causing a stack-buffer-overflow.

Fix: Clamp FormatLen1 and FormatLen2 to sizeof(ScrubbedString) - 1 (255) immediately before any memcpy, ensuring the copy never exceeds the buffer boundary.


Fix #1547OS_FileSysStatVolume_Impl() uses wrong statvfs field for block size

Files: src/os/posix/src/os-impl-filesys.c, src/os/qnx/src/os-impl-filesys.c, src/os/rtems/src/os-impl-filesys.c

Root cause: After migrating to statvfs(), the POSIX, QNX, and RTEMS implementations still assign stat_buf.f_bsize to result->block_size. Per POSIX, f_bsize is the preferred I/O transfer block size while f_frsize is the fundamental file system block size used to compute actual storage (i.e. total_blocks * f_frsize = total bytes). On large drives these values are not guaranteed to be equal, causing incorrect free/total byte calculations.

Fix: Replace stat_buf.f_bsize with stat_buf.f_frsize in all three affected OS ports.


Fix #1516 — Path traversal via unchecked .. components in OS_FileSys_FindVirtMountPoint()

File: src/os/shared/src/osapi-filesys.c

Root cause: OS_FileSys_FindVirtMountPoint() performed only a prefix match against the virtual mount point. A path such as /mnt/abc/../secret would match the /mnt/abc mount point because the prefix check passes and target[mplen] == '/'. An attacker could craft a path that traverses outside the virtual mount boundary.

Fix: Before performing the prefix match, scan the target path for any /../ or trailing /.. sequence and return false immediately if found. Also rejects paths starting with ../.

Files Changed

File Issue Change
ut_assert/src/utassert.c #1550 Clamp FormatLen before memcpy
src/os/posix/src/os-impl-filesys.c #1547 f_bsizef_frsize
src/os/qnx/src/os-impl-filesys.c #1547 f_bsizef_frsize
src/os/rtems/src/os-impl-filesys.c #1547 f_bsizef_frsize
src/os/shared/src/osapi-filesys.c #1516 Reject paths with .. components

Test plan

  • UtAssert_STRINGBUF_EQ() with buffers longer than 255 bytes no longer causes stack-buffer-overflow
  • OS_FileSysStatVolume() returns correct byte counts on large drives where f_frsize != f_bsize
  • OS_FileSys_FindVirtMountPoint() returns false for paths containing /../ or .. traversal
  • Existing unit tests pass with no regressions

…wrong statvfs block size field (nasa#1547), and path traversal in OS_FileSys_FindVirtMountPoint (nasa#1516)

- utassert.c: clamp FormatLen1/FormatLen2 to sizeof(ScrubbedString)-1
  before memcpy to prevent stack-buffer-overflow when caller-supplied
  String1Max/String2Max exceeds the 256-byte scrub buffers
- os-impl-filesys.c (posix, qnx, rtems): use f_frsize instead of
  f_bsize as the block size when reporting statvfs results; f_frsize
  is the fundamental block size that correctly represents the physical
  size of storage blocks, while f_bsize is the preferred I/O transfer
  size and is not guaranteed to equal f_frsize on large drives
- osapi-filesys.c: reject any virtual-mount-point path that contains
  a '..' component before prefix-matching to prevent path traversal
  attacks that could escape the virtual mount point boundary
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants