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

[Gang Hu]

Hi,

We're building a bug-finding tool and it flags a potential issue in
the useradd utility. In set_defaults() in src/useradd.c, the old
defaults file is renamed to the backup file, and the temporary file is
renamed to the defaults file. If there is a crash between these two
rename operations, then the link to the defaults file may be lost.
Although the user may recover it from the backup file, he may not
notice the problem, and he may use the utilities without the defaults
file. This may cause unexpected problems.  Here is the code snippet
with our  comments

      // shadow-4.1.5, file src/useradd.c

560:  if ((rename (USER_DEFAULTS_FILE, buf) != 0) && (ENOENT != errno)) {
         // [CHECKER] first rename
          ...
      }
         <===========================
          [CHECKER] crash here loses link to USER_DEFAULTS_FILE
      /*
       * Rename the new default file to its correct name.
       */
572:    if (rename (new_file, USER_DEFAULTS_FILE) != 0) {
        // [CHECKER] second rename

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?

--
Cheers,
Gang