Skip to content

Fix executable path truncation and copied-file error message - #272

Draft
rootkiller6788 wants to merge 1 commit into
facebookresearch:mainfrom
rootkiller6788:fix-exe-path-truncation-and-copy-err-msg
Draft

Fix executable path truncation and copied-file error message#272
rootkiller6788 wants to merge 1 commit into
facebookresearch:mainfrom
rootkiller6788:fix-exe-path-truncation-and-copy-err-msg

Conversation

@rootkiller6788

Copy link
Copy Markdown

Summary

Two small error-handling fixes in the vrs tools, both verified against current main (a6a3ad9).

1. os::Utils::getLinuxSelfExePath() — wrong buffer-size comparison, potential out-of-bounds write

getLinuxSelfExePath() compared readlink()'s return value against sizeof(buf). Inside the function buf is a char*, so sizeof(buf) is the pointer size (8 on 64-bit), not the caller-provided buffer length buflen. On Linux/Android this caused two problems:

  • A process whose executable path is exactly 8 characters long was treated as an error, making getCurrentExecutablePath() return an empty string (used for temp-folder naming).
  • When readlink() fills the whole buffer (path length >= PATH_MAX, i.e. a truncated path), the returned length equals buflen, and buf[len] = '\0' wrote one byte past the end of the caller's char exePath[PATH_MAX] stack buffer.

The fix compares against buflen and treats a full-buffer read (truncated path) as a failure, so a terminating null is never written past the buffer.

2. VrsCommand::doCopyMerge() — wrong path in "could not open copied file" error

When re-opening the output file after a copy/merge failed (outputFile.openFile(targetPath)), the error message printed the source file's path (filteredReader.getPathOrUri()) instead of the destination path that actually failed to open. It now reports targetPath.

Verification

  • getLinuxSelfExePath boundary behavior (readlink error, exactly-8-char path, full-buffer truncation) checked against the old vs new logic; the change is Linux/Android-only and leaves the Windows/macOS paths untouched.
  • VrsCommand::doCopyMerge error path: the reported path now matches the file passed to outputFile.openFile().

getLinuxSelfExePath() compared readlink()'s return value against sizeof(buf),
which for a char* parameter is the pointer size, not the buffer length. A
path exactly 8 characters long was treated as an error, and a truncated path
(one that fills the buffer) wrote a terminating null one byte past the end
of the buffer. Compare against buflen instead, and treat a full-buffer read
as a failure.

Also, when the copied output file fails to open, VrsCommand reported the
source file's path instead of the destination path that actually failed.
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant