[LTP] [PATCH v2] tst_tmpdir: Fix buffer overflow in tst_tmpdir.c

Wei Gao via ltp
2025-05-23T09:10:23Z

Using sprintf without length checking in tst_tmpdir may lead to buffer overflow.
So in this patch use openat() instead of open().

Fixes: https://github.com/linux-test-project/ltp/issues/1241
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Wei Gao <wegao@suse.com>
---
lib/tst_tmpdir.c | 122 ++++++++++++++++++++++++++++++-----------------
1 file changed, 78 insertions(+), 44 deletions(-)

diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 6ed2367b9..bcb9534c5 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -101,7 +101,7 @@ static char test_start_work_dir[PATH_MAX];
/* lib/tst_checkpoint.c */
extern futex_t *tst_futexes;

-static int rmobj(const char *obj, char **errmsg);
+static int rmobjat(int dir_fd, const char *obj, char **errmsg);

int tst_tmpdir_created(void)
{
@@ -149,8 +149,8 @@ static int purge_dir(const char *path, char **errptr)
int ret_val = 0;
DIR *dir;
struct dirent *dir_ent;
- char dirobj[PATH_MAX];
static char err_msg[PATH_MAX + 1280];
+ int dir_fd = -1;

/* Do NOT perform the request if the directory is "/" */
if (!strcmp(path, "/")) {
@@ -167,7 +167,7 @@ static int purge_dir(const char *path, char **errptr)
/* Open the directory to get access to what is in it */
if (!(dir = opendir(path))) {
if (errptr) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"Cannot open directory %s; errno=%d: %s",
path, errno, tst_strerrno(errno));
*errptr = err_msg;
@@ -175,6 +175,68 @@ static int purge_dir(const char *path, char **errptr)
return -1;
}

+ dir_fd = dirfd(dir);
+ if (dir_fd == -1) {
+ closedir(dir);
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot get file descriptor for directory %s; errno=%d: %s",
+ path, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
+ /* Loop through the entries in the directory, removing each one */
+ for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
+ /* Don't remove "." or ".." */
+ if (!strcmp(dir_ent->d_name, ".")
+ || !strcmp(dir_ent->d_name, ".."))
+ continue;
+
+ /* Recursively remove the current entry */
+ if (rmobjat(dir_fd, dir_ent->d_name, errptr) != 0)
+ ret_val = -1;
+ }
+
+ closedir(dir);
+ return ret_val;
+}
+
+static int purge_dirat(int dir_fd, const char *path, char **errptr)
+{
+ int ret_val = 0;
+ DIR *dir;
+ struct dirent *dir_ent;
+ static char err_msg[PATH_MAX + 1280];
+ int subdir_fd;
+
+ errno = 0;
+
+ /* Open the subdirectory using openat */
+ subdir_fd = openat(dir_fd, path, O_RDONLY | O_DIRECTORY | O_NOFOLLOW);
+ if (subdir_fd < 0) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open subdirectory %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ return -1;
+ }
+
+ dir = fdopendir(subdir_fd);
+ if (!dir) {
+ if (errptr) {
+ snprintf(err_msg, sizeof(err_msg),
+ "Cannot open directory stream for %s (via fd %d); errno=%d: %s",
+ path, dir_fd, errno, tst_strerrno(errno));
+ *errptr = err_msg;
+ }
+ close(subdir_fd);
+ return -1;
+ }
+
/* Loop through the entries in the directory, removing each one */
for (dir_ent = readdir(dir); dir_ent; dir_ent = readdir(dir)) {
/* Don't remove "." or ".." */
@@ -183,8 +245,7 @@ static int purge_dir(const char *path, char **errptr)
continue;

/* Recursively remove the current entry */
- sprintf(dirobj, "%s/%s", path, dir_ent->d_name);
- if (rmobj(dirobj, errptr) != 0)
+ if (rmobjat(subdir_fd, dir_ent->d_name, errptr) != 0)
ret_val = -1;
}

@@ -192,63 +253,36 @@ static int purge_dir(const char *path, char **errptr)
return ret_val;
}

-static int rmobj(const char *obj, char **errmsg)
+static int rmobjat(int dir_fd, const char *obj, char **errmsg)
{
int ret_val = 0;
- struct stat statbuf;
static char err_msg[PATH_MAX + 1280];
int fd;

- fd = open(obj, O_DIRECTORY | O_NOFOLLOW);
+ fd = openat(dir_fd, obj, O_DIRECTORY | O_NOFOLLOW);
if (fd >= 0) {
close(fd);
- ret_val = purge_dir(obj, errmsg);
+ ret_val = purge_dirat(dir_fd, obj, errmsg);

/* If there were problems removing an entry, don't attempt to
remove the directory itself */
if (ret_val == -1)
return -1;

- /* Get the link count, now that all the entries have been removed */
- if (lstat(obj, &statbuf) < 0) {
+ if (unlinkat(dir_fd, obj, AT_REMOVEDIR) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
- "lstat(%s) failed; errno=%d: %s", obj,
- errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
- }
-
- /* Remove the directory itself */
- if (statbuf.st_nlink >= 3) {
- /* The directory is linked; unlink() must be used */
- if (unlink(obj) < 0) {
- if (errmsg != NULL) {
- sprintf(err_msg,
- "unlink(%s) failed; errno=%d: %s",
- obj, errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
- }
- } else {
- /* The directory is not linked; remove() can be used */
- if (remove(obj) < 0) {
- if (errmsg != NULL) {
- sprintf(err_msg,
+ snprintf(err_msg, sizeof(err_msg),
"remove(%s) failed; errno=%d: %s",
obj, errno, tst_strerrno(errno));
- *errmsg = err_msg;
- }
- return -1;
+ *errmsg = err_msg;
}
+ return -1;
}
} else {
- if (unlink(obj) < 0) {
+ if (unlinkat(dir_fd, obj, 0) < 0) {
if (errmsg != NULL) {
- sprintf(err_msg,
- "unlink(%s) failed; errno=%d: %s", obj,
+ snprintf(err_msg, sizeof(err_msg),
+ "unlinkat(%s) failed; errno=%d: %s", obj,
errno, tst_strerrno(errno));
*errmsg = err_msg;
}
@@ -305,7 +339,7 @@ void tst_tmpdir(void)
tst_resm(TERRNO, "%s: chdir(%s) failed", __func__, TESTDIR);

/* Try to remove the directory */
- if (rmobj(TESTDIR, &errmsg) == -1) {
+ if (rmobjat(0, TESTDIR, &errmsg) == -1) {
tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
__func__, TESTDIR, errmsg);
}
@@ -343,7 +377,7 @@ void tst_rmdir(void)
/*
* Attempt to remove the "TESTDIR" directory, using rmobj().
*/
- if (rmobj(TESTDIR, &errmsg) == -1) {
+ if (rmobjat(0, TESTDIR, &errmsg) == -1) {
tst_resm(TWARN, "%s: rmobj(%s) failed: %s",
__func__, TESTDIR, errmsg);
}
--
2.49.0