From feaa53781e3154ec913e83673ce1bd113878901b Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Wed, 14 Jan 2026 14:13:52 +0100 Subject: [PATCH 1/3] lib/fs/copy/: fcopy(): Add function to copy the contents of a file into another We could probably implement this more efficiently, but this is simple and works. It is based on code from lib/commonio.c:create_backup(). Verify that the destination file is empty before writing to it. All callers have an empty file, but being a generic API, let's verify anyway. If we didn't verify, and the file was larger than the source file, then we'd end up with a mix of the old and new contents. Signed-off-by: Alejandro Colomar --- lib/Makefile.am | 2 ++ lib/fs/copy/fcopy.c | 12 ++++++++++++ lib/fs/copy/fcopy.h | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 lib/fs/copy/fcopy.c create mode 100644 lib/fs/copy/fcopy.h diff --git a/lib/Makefile.am b/lib/Makefile.am index 6a9a164433..5be8dc9459 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -83,6 +83,8 @@ libshadow_la_SOURCES = \ find_new_uid.c \ find_new_sub_gids.c \ find_new_sub_uids.c \ + fs/copy/fcopy.c \ + fs/copy/fcopy.h \ fs/mkstemp/fmkomstemp.c \ fs/mkstemp/fmkomstemp.h \ fs/mkstemp/mkomstemp.c \ diff --git a/lib/fs/copy/fcopy.c b/lib/fs/copy/fcopy.c new file mode 100644 index 0000000000..aa4106af4e --- /dev/null +++ b/lib/fs/copy/fcopy.c @@ -0,0 +1,12 @@ +// SPDX-FileCopyrightText: 2026, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#include "config.h" + +#include "fs/copy/fcopy.h" + +#include + + +extern inline int fcopy(FILE *dst, FILE *src); diff --git a/lib/fs/copy/fcopy.h b/lib/fs/copy/fcopy.h new file mode 100644 index 0000000000..5fd89e0fca --- /dev/null +++ b/lib/fs/copy/fcopy.h @@ -0,0 +1,40 @@ +// SPDX-FileCopyrightText: 2026, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#ifndef SHADOW_INCLUDE_LIB_FS_COPY_FCOPY_H_ +#define SHADOW_INCLUDE_LIB_FS_COPY_FCOPY_H_ + + +#include "config.h" + +#include + + +inline int fcopy(FILE *dst, FILE *src); + + +// fcopy - FILE copy +inline int +fcopy(FILE *dst, FILE *src) +{ + int c; + + if (ftello(dst) != 0) + return -1; + if (fseek(src, 0, SEEK_SET) == -1) + return -1; + + while (EOF != (c = fgetc(src))) { + if (fputc(c, dst) == EOF) + return -1; + } + if (ferror(src)) + return -1; + if (fflush(dst) == -1) + return -1; + return 0; +} + + +#endif // include guard From 95867af2115e8a46192dc6e24984d54eb6152b11 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Wed, 14 Jan 2026 14:21:39 +0100 Subject: [PATCH 2/3] lib/, src/: Use fcopy() instead of its pattern No changes intended. Signed-off-by: Alejandro Colomar --- lib/commonio.c | 13 ++----------- src/vipw.c | 11 ++--------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/lib/commonio.c b/lib/commonio.c index 28dc7324cf..0bfd31b9dd 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -27,6 +27,7 @@ #include "atoi/getnum.h" #include "commonio.h" #include "defines.h" +#include "fs/copy/fcopy.h" #include "fs/mkstemp/fmkomstemp.h" #include "nscd.h" #ifdef WITH_TCB @@ -247,7 +248,6 @@ static int create_backup (const char *name, FILE * fp) struct stat sb; struct utimbuf ub; FILE *bkfp; - int c; stprintf_a(tmpf, "%s.cioXXXXXX", name); if (fstat (fileno (fp), &sb) != 0) { @@ -259,16 +259,7 @@ static int create_backup (const char *name, FILE * fp) return -1; } - /* TODO: faster copy, not one-char-at-a-time. --marekm */ - c = 0; - if (fseek (fp, 0, SEEK_SET) == 0) { - while ((c = getc (fp)) != EOF) { - if (putc (c, bkfp) == EOF) { - break; - } - } - } - if ((c != EOF) || (ferror (fp) != 0) || (fflush (bkfp) != 0)) { + if (fcopy(bkfp, fp) == -1) { (void) fclose (bkfp); unlink(tmpf); return -1; diff --git a/src/vipw.c b/src/vipw.c index f6a3b72eee..fbeb5bbb4d 100644 --- a/src/vipw.c +++ b/src/vipw.c @@ -43,6 +43,7 @@ #endif /* WITH_TCB */ #include "shadowlog.h" #include "sssd.h" +#include "fs/copy/fcopy.h" #include "fs/mkstemp/fmkomstemp.h" #include "string/sprintf/aprintf.h" #include "string/sprintf/snprintf.h" @@ -108,21 +109,13 @@ static int create_backup_file (FILE * fp, char *backup, struct stat *sb) { struct utimbuf ub; FILE *bkfp; - int c; bkfp = fmkomstemp(backup, 0, 0600); if (NULL == bkfp) { return -1; } - c = 0; - if (fseeko (fp, 0, SEEK_SET) == 0) - while ((c = getc (fp)) != EOF) { - if (putc (c, bkfp) == EOF) { - break; - } - } - if ((EOF != c) || (ferror (fp) != 0) || (fflush (bkfp) != 0)) { + if (fcopy(bkfp, fp) == -1) { fclose (bkfp); unlink (backup); return -1; From 26114cb922cda8f78d64547f6b97e289e330581d Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Wed, 14 Jan 2026 21:32:01 +0100 Subject: [PATCH 3/3] src/vipw.c: create_backup_file(): Use mask for mode_t This is consistent with fmkstemp_set_perms() in lib/commonio.c. This avoids accidentally granting dangerous permissions. Closes: Reported-by: Alejandro Colomar Suggested-by: Tobias Stoeckmann Signed-off-by: Alejandro Colomar --- src/vipw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vipw.c b/src/vipw.c index fbeb5bbb4d..27c062d771 100644 --- a/src/vipw.c +++ b/src/vipw.c @@ -130,7 +130,7 @@ static int create_backup_file (FILE * fp, char *backup, struct stat *sb) ub.modtime = sb->st_mtime; if ( (utime (backup, &ub) != 0) || (fchown(fileno(bkfp), sb->st_uid, sb->st_gid) != 0) - || (fchmod(fileno(bkfp), sb->st_mode) != 0)) { + || (fchmod(fileno(bkfp), sb->st_mode & 0664) != 0)) { fclose(bkfp); unlink (backup); return -1;