[Pkg-shadow-devel] Potential issue in the useradd utility

[Nicolas François]

Hi,

On Sun, Apr 22, 2012 at 09:35:18PM -0400, ganghu at cs.columbia.edu wrote:
> 
> A better way may be first link() the old file to the backup file, and
> then rename() the new file to the target file. It is written in this
> way in vipwedit() in src/vipw.c, so maybe in set_defaults() it should
> also be done in this way.
> 
> Is this a real problem?

Thanks for the report.

I've applied the following patch:

--- src/useradd.c	(révision 3719)
+++ src/useradd.c	(copie de travail)
@@ -557,11 +557,12 @@
 	 */
 	wlen = snprintf (buf, sizeof buf, "%s-", USER_DEFAULTS_FILE);
 	assert (wlen < (int) sizeof buf);
-	if ((rename (USER_DEFAULTS_FILE, buf) != 0) && (ENOENT != errno)) {
+	unlink (buf);
+	if ((link (USER_DEFAULTS_FILE, buf) != 0) && (ENOENT != errno)) {
 		int err = errno;
 		fprintf (stderr,
-		         _("%s: rename: %s: %s\n"),
-		         Prog, USER_DEFAULTS_FILE, strerror (err));
+		         _("%s: Cannot create backup file (%s): %s\n"),
+		         Prog, buf, strerror (err));
 		unlink (new_file);
 		return -1;
 	}

Best Regards,
-- 
Nekral