diff --git a/SOURCES/libuser-CVE-2015-3246.patch b/SOURCES/libuser-CVE-2015-3246.patch new file mode 100644 index 0000000..ca58b47 --- /dev/null +++ b/SOURCES/libuser-CVE-2015-3246.patch @@ -0,0 +1,1530 @@ +2015-06-26 Miloslav Trmač + + * modules/files.c (open_and_copy_file): Replace and heavily modify ... + (lu_files_create_backup): ... this. + (lock_file_handle_existing, lock_file_create, lock_file_remove) + (struct editing, editing_open, replace_file_or_symlink) + (editing_close): New functions. + (generic_lookup, generic_is_locked, lu_files_enumerate) + (lu_files_users_enumerate_by_group, lu_files_groups_enumerate_by_user) + (lu_files_enumerate_full): Remove locking on read-only operations. + (generic_add, generic_mod, generic_del, generic_lock) + (generic_setpass): Use struct editing instead of dealing with locking, + backups, SELinux individually. + + * lib/user_private.h (lu_util_lock_obtain, lu_util_lock_free): Mark + as deprecated. + + * lib/util.c (lu_util_field_write): Fail on an incomplete write(). + +2015-06-25 Miloslav Trmač + + * modules/files.c (format_generic, generic_setpass): Refuse to write + field values which contain \n. + * tests/files_test.py (Tests.testUserAdd9, Tests.testUserMod8) + (tests.testUserSetpass5, tests.testGroupAdd6, tests.testGroupMod7) + (tests.testGroupSetpass4): New tests. + +diff -up libuser-0.60/lib/user_private.h.CVE-2015-3246 libuser-0.60/lib/user_private.h +--- libuser-0.60/lib/user_private.h.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200 ++++ libuser-0.60/lib/user_private.h 2015-07-08 15:15:14.060544103 +0200 +@@ -330,9 +330,11 @@ typedef char lu_security_context_t; /* " + ((void)(PATH), (void)(MODE), (void)(ERROR), TRUE) + #endif + +-/* Lock a file. */ ++#ifndef LU_DISABLE_DEPRECATED ++/* Lock a file. Deprecated. */ + gpointer lu_util_lock_obtain(int fd, struct lu_error **error); + void lu_util_lock_free(gpointer lock); ++#endif + + /* Manipulate a colon-delimited flat text file. */ + char *lu_util_line_get_matching1(int fd, const char *firstpart, +diff -up libuser-0.60/lib/util.c.CVE-2015-3246 libuser-0.60/lib/util.c +--- libuser-0.60/lib/util.c.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200 ++++ libuser-0.60/lib/util.c 2015-07-08 15:15:14.060544103 +0200 +@@ -632,7 +632,7 @@ lu_util_field_write(int fd, const char * + goto err; + } + len = strlen(buf); +- if (write(fd, buf, len) == -1) { ++ if (write(fd, buf, len) != len) { + lu_error_new(error, lu_error_write, NULL); + ret = FALSE; + goto err; +diff -up libuser-0.60/modules/files.c.CVE-2015-3246 libuser-0.60/modules/files.c +--- libuser-0.60/modules/files.c.CVE-2015-3246 2013-10-12 23:56:07.000000000 +0200 ++++ libuser-0.60/modules/files.c 2015-07-08 15:16:41.014981429 +0200 +@@ -25,6 +25,7 @@ + #include + #include + #include ++#include + #include + #include + #include +@@ -101,82 +102,79 @@ module_filename(struct lu_module *module + return g_strconcat(dir, file_suffix, NULL); + } + +-/* Create a backup copy of "filename" named "filename-". */ +-static gboolean +-lu_files_create_backup(const char *filename, +- struct lu_error **error) ++/* Copy contents of INPUT_FILENAME to OUTPUT_FILENAME, exclusively creating it ++ * if EXCLUSIVE. ++ * Return the file descriptor for OUTPUT_FILENAME, open for reading and writing, ++ * or -1 on error. ++ * Note that this does no locking and assumes the directories hosting the files ++ * are not being manipulated by an attacker. */ ++static int ++open_and_copy_file(const char *input_filename, const char *output_filename, ++ gboolean exclusive, struct lu_error **error) + { + int ifd, ofd; +- gpointer ilock, olock; +- char *backupname; +- struct stat ist, ost; +- off_t offset; +- gboolean res = FALSE; ++ struct stat st; ++ int res = -1; ++ int flags; + +- g_assert(filename != NULL); +- g_assert(strlen(filename) > 0); ++ g_assert(input_filename != NULL); ++ g_assert(strlen(input_filename) > 0); ++ g_assert(output_filename != NULL); ++ g_assert(strlen(output_filename) > 0); + +- /* Open the original file. */ +- ifd = open(filename, O_RDONLY); ++ /* Open the input file. */ ++ ifd = open(input_filename, O_RDONLY); + if (ifd == -1) { + lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, ++ _("couldn't open `%s': %s"), input_filename, + strerror(errno)); + goto err; + } + +- /* Lock the input file. */ +- if ((ilock = lu_util_lock_obtain(ifd, error)) == NULL) +- goto err_ifd; +- + /* Read the input file's size. */ +- if (fstat(ifd, &ist) == -1) { ++ if (fstat(ifd, &st) == -1) { + lu_error_new(error, lu_error_stat, +- _("couldn't stat `%s': %s"), filename, ++ _("couldn't stat `%s': %s"), input_filename, + strerror(errno)); +- goto err_ilock; ++ goto err_ifd; + } + +- /* Generate the backup file's name and open it, creating it if it +- * doesn't already exist. */ +- backupname = g_strconcat(filename, "-", NULL); +- ofd = open(backupname, O_WRONLY | O_CREAT, ist.st_mode); ++ /* We only need O_WRONLY, but the caller needs RDWR if ofd will be ++ * used as e->new_fd. */ ++ flags = O_RDWR | O_CREAT; ++ if (exclusive) { ++ /* This ensures that if there is a concurrent writer which is ++ * not doing locking for some reason, we will not truncate their ++ * temporary file. Still, the other writer may truncate our ++ * file, and ultimately the rename() committing the changes will ++ * lose one or the other set of changes. */ ++ (void)unlink(output_filename); ++ flags |= O_EXCL; ++ } else ++ flags |= O_TRUNC; ++ /* Start with absolutely restrictive permissions to make sure nobody ++ * can get a file descriptor for this file until we are done resetting ++ * ownership. */ ++ ofd = open(output_filename, flags, 0); + if (ofd == -1) { + lu_error_new(error, lu_error_open, +- _("error creating `%s': %s"), backupname, ++ _("error creating `%s': %s"), output_filename, + strerror(errno)); +- goto err_backupname; +- } +- +- /* If we can't read its type, or it's not a normal file, bail. */ +- if (fstat(ofd, &ost) == -1) { +- lu_error_new(error, lu_error_stat, _("couldn't stat `%s': %s"), +- backupname, strerror(errno)); +- goto err_ofd; +- } +- if (!S_ISREG(ost.st_mode)) { +- lu_error_new(error, lu_error_open, +- _("backup file `%s' exists and is not a regular file"), +- backupname); +- goto err_ofd; ++ goto err_ifd; + } + +- /* Now lock the output file. */ +- if ((olock = lu_util_lock_obtain(ofd, error)) == NULL) +- goto err_ofd; +- + /* Set the permissions on the new file to match the old one. */ +- if (fchown(ofd, ist.st_uid, ist.st_gid) == -1 && errno != EPERM) { ++ if (fchown(ofd, st.st_uid, st.st_gid) == -1 && errno != EPERM) { + lu_error_new(error, lu_error_generic, +- _("Error changing owner of `%s': %s"), backupname, +- strerror(errno)); +- goto err_olock; ++ _("Error changing owner of `%s': %s"), ++ output_filename, strerror(errno)); ++ goto err_ofd; + } +- if (fchmod(ofd, ist.st_mode) == -1) { ++ if (fchmod(ofd, st.st_mode) == -1) { + lu_error_new(error, lu_error_generic, +- _("Error changing mode of `%s': %s"), backupname, +- strerror(errno)); +- goto err_olock; ++ _("Error changing mode of `%s': %s"), ++ output_filename, strerror(errno)); ++ goto err_ofd; + } + + /* Copy the data, block by block. */ +@@ -190,9 +188,9 @@ lu_files_create_backup(const char *filen + if (errno == EINTR) + continue; + lu_error_new(error, lu_error_read, +- _("Error reading `%s': %s"), filename, +- strerror(errno)); +- goto err_olock; ++ _("Error reading `%s': %s"), ++ input_filename, strerror(errno)); ++ goto err_ofd; + } + if (left == 0) + break; +@@ -206,55 +204,297 @@ lu_files_create_backup(const char *filen + continue; + lu_error_new(error, lu_error_write, + _("Error writing `%s': %s"), +- backupname, strerror(errno)); +- goto err_olock; ++ output_filename, strerror(errno)); ++ goto err_ofd; + } + p += out; + left -= out; + } + } + +- /* Flush data to disk, and truncate at the current offset. This is +- * necessary if the file existed before we opened it. */ +- fsync(ofd); +- offset = lseek(ofd, 0, SEEK_CUR); +- if (offset == -1 || ftruncate(ofd, offset) == -1) { +- lu_error_new(error, lu_error_generic, +- _("Error writing `%s': %s"), backupname, +- strerror(errno)); +- goto err_olock; +- } +- +- /* Re-read data about the output file. */ +- if (fstat(ofd, &ost) == -1) { +- lu_error_new(error, lu_error_stat, +- _("couldn't stat `%s': %s"), backupname, +- strerror(errno)); +- goto err_olock; +- } +- +- /* Complain if the files are somehow not the same. */ +- if (ist.st_size != ost.st_size) { +- lu_error_new(error, lu_error_generic, +- _("backup file size mismatch")); +- goto err_olock; ++ /* Flush data to disk. */ ++ if (fsync(ofd) != 0 || lseek(ofd, 0, SEEK_SET) == -1) { ++ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"), ++ output_filename, strerror(errno)); ++ goto err_ofd; + } +- res = TRUE; ++ res = ofd; ++ goto err_ifd; /* Do not close ofd */ + +- err_olock: +- lu_util_lock_free(olock); + err_ofd: + close(ofd); +- err_backupname: +- g_free(backupname); +- err_ilock: +- lu_util_lock_free(ilock); + err_ifd: + close(ifd); + err: + return res; + } + ++/* Deal with an existing LOCK_FILENAME. ++ * Return TRUE if the caller should try again. */ ++static gboolean ++lock_file_handle_existing(const char *lock_filename, struct lu_error **error) ++{ ++ gchar *lock_contents; ++ GError *gerror; ++ gboolean ret = FALSE; ++ uintmax_t pid; ++ char *p; ++ ++ gerror = NULL; ++ if (g_file_get_contents(lock_filename, &lock_contents, NULL, &gerror) ++ == FALSE) { ++ lu_error_new(error, lu_error_read, ++ _("couldn't read from `%s': %s"), lock_filename, ++ gerror->message); ++ g_error_free(gerror); ++ goto err; ++ } ++ errno = 0; ++ pid = strtoumax(lock_contents, &p, 10); ++ if (errno != 0 || *p != 0 || p == lock_contents || (pid_t)pid != pid) { ++ lu_error_new(error, lu_error_lock, ++ _("Invalid contents of lock `%s'"), lock_filename); ++ goto err_lock_contents; ++ } ++ if (kill(pid, 0) == 0 || errno != ESRCH) { ++ lu_error_new(error, lu_error_lock, ++ _("The lock %s is held by process %ju"), ++ lock_filename, pid); ++ goto err_lock_contents; ++ } ++ /* This is unfixably racy, but that should matter only if a genuine ++ * lock owner crashes. */ ++ if (unlink(lock_filename) != 0) { ++ lu_error_new(error, lu_error_lock, ++ _("Error removing stale lock `%s': %s"), lock_filename, ++ strerror(errno)); ++ goto err_lock_contents; ++ } ++ ret = TRUE; ++ /* Fall through */ ++ ++err_lock_contents: ++ g_free(lock_contents); ++err: ++ return ret; ++} ++ ++/* Create a lock file for FILENAME. */ ++static gboolean ++lock_file_create(const char *filename, struct lu_error **error) ++{ ++ char *lock_filename, *tmp_filename; ++ char pid_string[sizeof (pid_t) * CHAR_BIT + 1]; ++ int fd; ++ gboolean ret = FALSE; ++ ++ lock_filename = g_strconcat(filename, ".lock", NULL); ++ tmp_filename = g_strdup_printf("%s.lock.XXXXXX", filename); ++ ++ fd = mkstemp(tmp_filename); ++ if (fd == -1) { ++ lu_error_new(error, lu_error_open, ++ _("error opening temporary file for `%s': %s"), ++ lock_filename, strerror(errno)); ++ goto err_tmp_filename; ++ } ++ if (snprintf(pid_string, sizeof(pid_string), "%ju", (uintmax_t)getpid()) ++ >= sizeof(pid_string)) ++ g_assert_not_reached(); ++ if (write(fd, pid_string, strlen(pid_string)) != strlen(pid_string)) { ++ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"), ++ tmp_filename, strerror(errno)); ++ close(fd); ++ goto err_tmp_file; ++ } ++ close(fd); ++ ++ if (link(tmp_filename, lock_filename) != 0) { ++ if (errno == EEXIST) { ++ if (lock_file_handle_existing(lock_filename, error) ++ == FALSE) ++ goto err_tmp_file; ++ if (link(tmp_filename, lock_filename) == 0) ++ goto got_link; ++ } ++ lu_error_new(error, lu_error_lock, ++ _("Cannot obtain lock `%s': %s"), lock_filename, ++ strerror(errno)); ++ goto err_tmp_file; ++ } ++got_link: ++ ret = TRUE; ++ /* Fall through */ ++ ++err_tmp_file: ++ (void)unlink(tmp_filename); ++err_tmp_filename: ++ g_free(tmp_filename); ++ g_free(lock_filename); ++ return ret; ++} ++ ++/* Remove the lock file for FILENAME. */ ++static void ++lock_file_remove(const char *filename) ++{ ++ char *lock_file; ++ ++ lock_file = g_strconcat(filename, ".lock", NULL); ++ (void)unlink(lock_file); ++ g_free(lock_file); ++} ++ ++/* State related to a file currently open for editing. */ ++struct editing { ++ char *filename; ++ lu_security_context_t fscreate; ++ char *new_filename; ++ int new_fd; ++}; ++ ++/* Open and lock FILE_SUFFIX in MODULE for editing. ++ * Return editing state, or NULL on error. */ ++static struct editing * ++editing_open(struct lu_module *module, const char *file_suffix, ++ struct lu_error **error) ++{ ++ struct editing *e; ++ char *backup_name; ++ int fd; ++ ++ e = g_malloc0(sizeof (*e)); ++ e->filename = module_filename(module, file_suffix); ++ /* Make sure this all works if e->filename is a symbolic link, at least ++ * as long as it points to the same file system. */ ++ ++ if (geteuid() == 0) { ++ if (lckpwdf() != 0) { ++ lu_error_new(error, lu_error_lock, ++ _("error locking file: %s"), ++ strerror(errno)); ++ goto err_filename; ++ } ++ } ++ if (lock_file_create(e->filename, error) == FALSE) ++ goto err_lckpwdf; ++ ++ if (!lu_util_fscreate_save(&e->fscreate, error)) ++ goto err_locked; ++ if (!lu_util_fscreate_from_file(e->filename, error)) ++ goto err_fscreate; ++ ++ backup_name = g_strconcat(e->filename, "-", NULL); ++ fd = open_and_copy_file(e->filename, backup_name, FALSE, error); ++ g_free (backup_name); ++ close(fd); ++ if (fd == -1) ++ goto err_fscreate; ++ ++ e->new_filename = g_strconcat(e->filename, "+", NULL); ++ e->new_fd = open_and_copy_file(e->filename, e->new_filename, TRUE, ++ error); ++ if (e->new_fd == -1) ++ goto err_new_filename; ++ ++ return e; ++ ++err_new_filename: ++ g_free(e->new_filename); ++err_fscreate: ++ lu_util_fscreate_restore(e->fscreate); ++ ++err_locked: ++ (void)lock_file_remove(e->filename); ++err_lckpwdf: ++ if (geteuid() == 0) ++ (void)ulckpwdf(); ++ ++err_filename: ++ g_free(e->filename); ++ g_free(e); ++ return NULL; ++} ++ ++ ++/* Replace DESTINATION with SOURCE, even if DESTINATION is a symbolic link. */ ++static gboolean ++replace_file_or_symlink(const char *source, const char *destination, ++ struct lu_error **error) ++{ ++ struct stat st; ++ char *tmp; ++ gboolean ret = FALSE; ++ ++ tmp = NULL; ++ if (lstat(destination, &st) == 0 && S_ISLNK(st.st_mode)) { ++ tmp = realpath(destination, NULL); ++ if (tmp == NULL) { ++ lu_error_new(error, lu_error_generic, ++ _("Error resolving `%s': %s"), destination, ++ strerror(errno)); ++ goto err; ++ } ++ destination = tmp; ++ } ++ if (rename(source, destination) != 0) { ++ lu_error_new(error, lu_error_write, ++ _("Error replacing `%s': %s"), destination, ++ strerror(errno)); ++ goto err; ++ } ++ ret = TRUE; ++ /* Fall through */ ++ ++err: ++ free(tmp); ++ return ret; ++} ++ ++/* Finish editing E, commit edits if COMMIT. ++ * Return true only if RET_INPUT and everything went OK; suggested usage is ++ * ret = editing_close(e, commit, ret, error); */ ++static gboolean ++editing_close(struct editing *e, gboolean commit, gboolean ret_input, ++ struct lu_error **error) ++{ ++ gboolean ret = FALSE; ++ gboolean unlink_new_filename = TRUE; ++ ++ g_assert(e != NULL); ++ ++ if (commit && fsync(e->new_fd) != 0) { ++ lu_error_new(error, lu_error_write, _("Error writing `%s': %s"), ++ e->new_filename, strerror(errno)); ++ close(e->new_fd); ++ goto err; ++ } ++ close(e->new_fd); ++ ++ if (commit) { ++ if (replace_file_or_symlink(e->new_filename, e->filename, ++ error) == FALSE) ++ goto err; ++ unlink_new_filename = FALSE; ++ } ++ ret = ret_input; ++ ++err: ++ if (unlink_new_filename) ++ (void)unlink(e->new_filename); ++ g_free(e->new_filename); ++ lu_util_fscreate_restore(e->fscreate); ++ ++ (void)lock_file_remove(e->filename); ++ if (geteuid() == 0) ++ (void)ulckpwdf(); ++ ++ g_free(e->filename); ++ g_free(e); ++ return ret; ++} ++ ++ + /* Read a line from the file, no matter how long it is, and return it as a + * newly-allocated string, with the terminator intact. */ + static char * +@@ -435,7 +675,6 @@ generic_lookup(struct lu_module *module, + { + gboolean ret; + int fd = -1; +- gpointer lock; + char *line, *filename; + + g_assert(module != NULL); +@@ -446,7 +685,7 @@ generic_lookup(struct lu_module *module, + + filename = module_filename(module, file_suffix); + +- /* Open the file and lock it. */ ++ /* Open the file. */ + fd = open(filename, O_RDONLY); + if (fd == -1) { + lu_error_new(error, lu_error_open, +@@ -457,15 +696,9 @@ generic_lookup(struct lu_module *module, + } + g_free(filename); + +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- return FALSE; +- } +- + /* Search for the entry in this file. */ + line = lu_util_line_get_matchingx(fd, name, field, error); + if (line == NULL) { +- lu_util_lock_free(lock); + close(fd); + return FALSE; + } +@@ -473,7 +706,6 @@ generic_lookup(struct lu_module *module, + /* If we found data, parse it and then free the data. */ + ret = parser(line, ent); + g_free(line); +- lu_util_lock_free(lock); + close(fd); + + return ret; +@@ -666,6 +898,13 @@ format_generic(struct lu_ent *ent, const + char *field; + + field = format_field(ent, formats + i); ++ if (strchr(field, '\n') != NULL) { ++ lu_error_new(error, lu_error_invalid_attribute_value, ++ _("%s value `%s': `\\n' not allowed"), ++ formats[i].attribute, field); ++ g_free(field); ++ goto err; ++ } + if (i != format_count - 1 && strchr(field, ':') != NULL) { + lu_error_new(error, lu_error_invalid_attribute_value, + _("%s value `%s': `:' not allowed"), +@@ -729,11 +968,9 @@ generic_add(struct lu_module *module, co + const struct format_specifier *formats, size_t format_count, + struct lu_ent *ent, struct lu_error **error) + { +- lu_security_context_t fscreate; +- char *line, *filename, *contents; +- int fd; ++ struct editing *e; ++ char *line, *contents; + ssize_t r; +- gpointer lock; + struct stat st; + off_t offset; + gboolean ret = FALSE; +@@ -743,50 +980,30 @@ generic_add(struct lu_module *module, co + g_assert(format_count > 0); + g_assert(ent != NULL); + +- filename = module_filename(module, file_suffix); +- + line = format_generic(ent, formats, format_count, error); + if (line == NULL) +- goto err_filename; ++ goto err; + +- if (!lu_util_fscreate_save(&fscreate, error)) ++ e = editing_open(module, file_suffix, error); ++ if (e == NULL) + goto err_line; +- if (!lu_util_fscreate_from_file(filename, error)) +- goto err_fscreate; +- +- /* Create a backup copy of the file we're about to modify. */ +- if (lu_files_create_backup(filename, error) == FALSE) +- goto err_fscreate; +- +- /* Open the file. */ +- fd = open(filename, O_RDWR); +- if (fd == -1) { +- lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, +- strerror(errno)); +- goto err_fscreate; +- } +- +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; + + /* Read the file's size. */ +- if (fstat(fd, &st) == -1) { ++ if (fstat(e->new_fd, &st) == -1) { + lu_error_new(error, lu_error_stat, +- _("couldn't stat `%s': %s"), filename, ++ _("couldn't stat `%s': %s"), e->new_filename, + strerror(errno)); +- goto err_lock; ++ goto err_editing; + } + + /* Read the entire file in. There's some room for improvement here, + * but at least we still have the lock, so it's not going to get + * funky on us. */ + contents = g_malloc0(st.st_size + 1); +- if (read(fd, contents, st.st_size) != st.st_size) { ++ if (read(e->new_fd, contents, st.st_size) != st.st_size) { + lu_error_new(error, lu_error_read, + _("couldn't read from `%s': %s"), +- filename, strerror(errno)); ++ e->new_filename, strerror(errno)); + goto err_contents; + } + +@@ -798,54 +1015,43 @@ generic_add(struct lu_module *module, co + goto err_contents; + } + /* Hooray, we can add this entry at the end of the file. */ +- offset = lseek(fd, 0, SEEK_END); ++ offset = lseek(e->new_fd, 0, SEEK_END); + if (offset == -1) { + lu_error_new(error, lu_error_write, + _("couldn't write to `%s': %s"), +- filename, strerror(errno)); ++ e->new_filename, strerror(errno)); + goto err_contents; + } + /* If the last byte in the file isn't a newline, add one, and silently + * curse people who use text editors (which shall remain unnamed) which + * allow saving of the file without a final line terminator. */ + if ((st.st_size > 0) && (contents[st.st_size - 1] != '\n')) { +- if (write(fd, "\n", 1) != 1) { ++ if (write(e->new_fd, "\n", 1) != 1) { + lu_error_new(error, lu_error_write, + _("couldn't write to `%s': %s"), +- filename, +- strerror(errno)); ++ e->new_filename, strerror(errno)); + goto err_contents; + } + } + /* Attempt to write the entire line to the end. */ +- r = write(fd, line, strlen(line)); ++ r = write(e->new_fd, line, strlen(line)); + if ((size_t)r != strlen(line)) { + /* Oh, come on! */ + lu_error_new(error, lu_error_write, +- _("couldn't write to `%s': %s"), +- filename, ++ _("couldn't write to `%s': %s"), e->new_filename, + strerror(errno)); +- /* Truncate off whatever we actually managed to write and +- * give up. */ +- (void)ftruncate(fd, offset); + goto err_contents; + } +- /* Hey, it succeeded. */ + ret = TRUE; + /* Fall through */ + + err_contents: + g_free(contents); +-err_lock: +- lu_util_lock_free(lock); +-err_fd: +- close(fd); +-err_fscreate: +- lu_util_fscreate_restore(fscreate); ++err_editing: ++ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */ + err_line: + g_free(line); +-err_filename: +- g_free(filename); ++err: + return ret; + } + +@@ -938,11 +1144,9 @@ generic_mod(struct lu_module *module, co + const struct format_specifier *formats, size_t format_count, + struct lu_ent *ent, struct lu_error **error) + { +- lu_security_context_t fscreate; +- char *filename, *new_line, *contents, *line, *rest; ++ struct editing *e; ++ char *new_line, *contents, *line, *rest; + char *current_name, *fragment; +- int fd; +- gpointer lock; + const char *name_attribute; + gboolean ret = FALSE; + struct stat st; +@@ -971,43 +1175,24 @@ generic_mod(struct lu_module *module, co + return FALSE; + } + +- filename = module_filename(module, file_suffix); +- + new_line = format_generic(ent, formats, format_count, error); + if (new_line == NULL) +- goto err_filename; ++ goto err_current_name; + +- if (!lu_util_fscreate_save(&fscreate, error)) ++ e = editing_open(module, file_suffix, error); ++ if (e == NULL) + goto err_new_line; +- if (!lu_util_fscreate_from_file(filename, error)) +- goto err_fscreate; +- /* Create a backup file. */ +- if (lu_files_create_backup(filename, error) == FALSE) +- goto err_fscreate; + +- /* Open the file to be modified. */ +- fd = open(filename, O_RDWR); +- if (fd == -1) { +- lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, +- strerror(errno)); +- goto err_fscreate; +- } +- +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; +- +- if (fstat(fd, &st) == -1) { ++ if (fstat(e->new_fd, &st) == -1) { + lu_error_new(error, lu_error_stat, _("couldn't stat `%s': %s"), +- filename, strerror(errno)); +- goto err_lock; ++ e->new_filename, strerror(errno)); ++ goto err_editing; + } + + contents = g_malloc(st.st_size + 1 + strlen(new_line)); +- if (read(fd, contents, st.st_size) != st.st_size) { ++ if (read(e->new_fd, contents, st.st_size) != st.st_size) { + lu_error_new(error, lu_error_read, +- _("couldn't read from `%s': %s"), filename, ++ _("couldn't read from `%s': %s"), e->new_filename, + strerror(errno)); + goto err_contents; + } +@@ -1045,16 +1230,16 @@ generic_mod(struct lu_module *module, co + memmove(line + strlen(new_line), rest, + contents + st.st_size + 1 - rest); + memcpy(line, new_line, strlen(new_line)); +- if (lseek(fd, line - contents, SEEK_SET) == -1) { ++ if (lseek(e->new_fd, line - contents, SEEK_SET) == -1) { + lu_error_new(error, lu_error_write, NULL); + goto err_contents; + } + len = strlen(line); +- if ((size_t)write(fd, line, len) != len) { ++ if ((size_t)write(e->new_fd, line, len) != len) { + lu_error_new(error, lu_error_write, NULL); + goto err_contents; + } +- if (ftruncate(fd, (line - contents) + len) != 0) { ++ if (ftruncate(e->new_fd, (line - contents) + len) != 0) { + lu_error_new(error, lu_error_write, NULL); + goto err_contents; + } +@@ -1063,16 +1248,11 @@ generic_mod(struct lu_module *module, co + + err_contents: + g_free(contents); +-err_lock: +- lu_util_lock_free(lock); +-err_fd: +- close(fd); +-err_fscreate: +- lu_util_fscreate_restore(fscreate); ++err_editing: ++ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */ + err_new_line: + g_free(new_line); +-err_filename: +- g_free(filename); ++err_current_name: + g_free(current_name); + return ret; + } +@@ -1118,16 +1298,14 @@ static gboolean + generic_del(struct lu_module *module, const char *file_suffix, + struct lu_ent *ent, struct lu_error **error) + { +- lu_security_context_t fscreate; ++ struct editing *e; + char *name; +- char *contents, *filename; ++ char *contents; + char *fragment2; + struct stat st; + size_t len; +- int fd; +- gboolean ret = FALSE; ++ gboolean commit = FALSE, ret = FALSE; + gboolean found; +- gpointer lock; + + /* Get the entity's current name. */ + if (ent->type == lu_user) +@@ -1141,42 +1319,23 @@ generic_del(struct lu_module *module, co + g_assert(module != NULL); + g_assert(ent != NULL); + +- filename = module_filename(module, file_suffix); +- +- if (!lu_util_fscreate_save(&fscreate, error)) +- goto err_filename; +- if (!lu_util_fscreate_from_file(filename, error)) +- goto err_fscreate; +- /* Create a backup of that file. */ +- if (lu_files_create_backup(filename, error) == FALSE) +- goto err_fscreate; +- +- /* Open the file to be modified. */ +- fd = open(filename, O_RDWR); +- if (fd == -1) { +- lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, +- strerror(errno)); +- goto err_fscreate; +- } +- +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; ++ e = editing_open(module, file_suffix, error); ++ if (e == NULL) ++ goto err_name; + + /* Determine the file's size. */ +- if (fstat(fd, &st) == -1) { ++ if (fstat(e->new_fd, &st) == -1) { + lu_error_new(error, lu_error_stat, +- _("couldn't stat `%s': %s"), filename, ++ _("couldn't stat `%s': %s"), e->new_filename, + strerror(errno)); +- goto err_lock; ++ goto err_editing; + } + + /* Allocate space to hold the file and read it all in. */ + contents = g_malloc(st.st_size + 1); +- if (read(fd, contents, st.st_size) != st.st_size) { ++ if (read(e->new_fd, contents, st.st_size) != st.st_size) { + lu_error_new(error, lu_error_read, +- _("couldn't read from `%s': %s"), filename, ++ _("couldn't read from `%s': %s"), e->new_filename, + strerror(errno)); + goto err_contents; + } +@@ -1229,41 +1388,38 @@ generic_del(struct lu_module *module, co + + /* Otherwise we need to write the new data to the file. Jump back to + * the beginning of the file. */ +- if (lseek(fd, 0, SEEK_SET) == -1) { ++ if (lseek(e->new_fd, 0, SEEK_SET) == -1) { + lu_error_new(error, lu_error_write, +- _("couldn't write to `%s': %s"), filename, ++ _("couldn't write to `%s': %s"), e->new_filename, + strerror(errno)); + goto err_contents; + } + + /* Write the new contents out. */ +- if ((size_t)write(fd, contents, len) != len) { ++ if ((size_t)write(e->new_fd, contents, len) != len) { + lu_error_new(error, lu_error_write, +- _("couldn't write to `%s': %s"), filename, ++ _("couldn't write to `%s': %s"), e->new_filename, + strerror(errno)); + goto err_contents; + } + + /* Truncate the file to the new (certainly shorter) length. */ +- if (ftruncate(fd, len) == -1) { ++ if (ftruncate(e->new_fd, len) == -1) { + lu_error_new(error, lu_error_generic, +- _("couldn't write to `%s': %s"), filename, ++ _("couldn't write to `%s': %s"), e->new_filename, + strerror(errno)); + goto err_contents; + } ++ commit = TRUE; + ret = TRUE; + /* Fall through */ + + err_contents: + g_free(contents); +- err_lock: +- lu_util_lock_free(lock); +- err_fd: +- close(fd); +-err_fscreate: +- lu_util_fscreate_restore(fscreate); +- err_filename: +- g_free(filename); ++err_editing: ++ /* Commit/rollback happens here. */ ++ ret = editing_close(e, commit, ret, error); ++err_name: + g_free(name); + return ret; + } +@@ -1344,12 +1500,9 @@ static gboolean + generic_lock(struct lu_module *module, const char *file_suffix, int field, + struct lu_ent *ent, enum lock_op op, struct lu_error **error) + { +- lu_security_context_t fscreate; +- char *filename; ++ struct editing *e; + char *value, *new_value, *name; +- int fd; +- gpointer lock; +- gboolean ret = FALSE; ++ gboolean commit = FALSE, ret = FALSE; + + /* Get the name which keys the entries of interest in the file. */ + g_assert((ent->type == lu_user) || (ent->type == lu_group)); +@@ -1362,33 +1515,14 @@ generic_lock(struct lu_module *module, c + g_assert(module != NULL); + g_assert(ent != NULL); + +- filename = module_filename(module, file_suffix); +- +- if (!lu_util_fscreate_save(&fscreate, error)) +- goto err_filename; +- if (!lu_util_fscreate_from_file(filename, error)) +- goto err_fscreate; +- /* Create a backup of the file. */ +- if (lu_files_create_backup(filename, error) == FALSE) +- goto err_fscreate; +- +- /* Open the file. */ +- fd = open(filename, O_RDWR); +- if (fd == -1) { +- lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, +- strerror(errno)); +- goto err_fscreate; +- } +- +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; ++ e = editing_open(module, file_suffix, error); ++ if (e == NULL) ++ goto err_name; + + /* Read the old value from the file. */ +- value = lu_util_field_read(fd, name, field, error); ++ value = lu_util_field_read(e->new_fd, name, field, error); + if (value == NULL) +- goto err_lock; ++ goto err_editing; + + /* Check that we actually care about this. If there's a non-empty, + * not locked string in there, but it's too short to be a hash, then +@@ -1396,27 +1530,27 @@ generic_lock(struct lu_module *module, c + if (LU_CRYPT_INVALID(value)) { + g_free(value); + ret = TRUE; +- goto err_lock; ++ goto err_editing; + } + + /* Generate a new value for the file. */ + new_value = lock_process(value, op, ent, error); + g_free(value); + if (new_value == NULL) +- goto err_lock; ++ goto err_editing; + + /* Make the change. */ +- ret = lu_util_field_write(fd, name, field, new_value, error); ++ if (lu_util_field_write(e->new_fd, name, field, new_value, error) ++ == FALSE) ++ goto err_editing; ++ commit = TRUE; ++ ret = TRUE; + /* Fall through */ + +-err_lock: +- lu_util_lock_free(lock); +- err_fd: +- close(fd); +-err_fscreate: +- lu_util_fscreate_restore(fscreate); +- err_filename: +- g_free(filename); ++err_editing: ++ /* Commit/rollback happens here. */ ++ ret = editing_close(e, commit, ret, error); ++err_name: + g_free(name); + return ret; + } +@@ -1429,7 +1563,6 @@ generic_is_locked(struct lu_module *modu + char *filename; + char *value, *name; + int fd; +- gpointer lock; + gboolean ret = FALSE; + + /* Get the name of this account. */ +@@ -1454,22 +1587,16 @@ generic_is_locked(struct lu_module *modu + goto err_filename; + } + +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; +- + /* Read the value. */ + value = lu_util_field_read(fd, name, field, error); + if (value == NULL) +- goto err_lock; ++ goto err_fd; + + /* It all comes down to this. */ + ret = value[0] == '!'; + g_free(value); + /* Fall through */ + +-err_lock: +- lu_util_lock_free(lock); + err_fd: + close(fd); + err_filename: +@@ -1624,10 +1751,8 @@ generic_setpass(struct lu_module *module + struct lu_ent *ent, const char *password, gboolean is_shadow, + struct lu_error **error) + { +- lu_security_context_t fscreate; +- char *filename, *value, *name; +- int fd; +- gpointer lock; ++ struct editing *e; ++ char *value, *name; + gboolean ret = FALSE; + + /* Get the name of this account. */ +@@ -1641,34 +1766,14 @@ generic_setpass(struct lu_module *module + g_assert(module != NULL); + g_assert(ent != NULL); + +- filename = module_filename(module, file_suffix); +- +- if (!lu_util_fscreate_save(&fscreate, error)) +- goto err_filename; +- if (!lu_util_fscreate_from_file(filename, error)) +- goto err_fscreate; +- +- /* Create a backup of the file. */ +- if (lu_files_create_backup(filename, error) == FALSE) +- goto err_filename; +- +- /* Open the file. */ +- fd = open(filename, O_RDWR); +- if (fd == -1) { +- lu_error_new(error, lu_error_open, +- _("couldn't open `%s': %s"), filename, +- strerror(errno)); +- goto err_fscreate; +- } +- +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) +- goto err_fd; ++ e = editing_open(module, file_suffix, error); ++ if (e == NULL) ++ goto err_name; + + /* Read the current contents of the field. */ +- value = lu_util_field_read(fd, name, field, error); ++ value = lu_util_field_read(e->new_fd, name, field, error); + if (value == NULL) +- goto err_lock; ++ goto err_editing; + + /* pam_unix uses shadow passwords only if pw_passwd is "x" + (or ##${username}). Make sure to preserve the shadow marker +@@ -1693,9 +1798,9 @@ generic_setpass(struct lu_module *module + else if (g_ascii_strncasecmp(password, LU_CRYPTED, strlen(LU_CRYPTED)) + == 0) { + password = password + strlen(LU_CRYPTED); +- if (strchr(password, ':') != NULL) { ++ if (strpbrk(password, ":\n") != NULL) { + lu_error_new(error, lu_error_invalid_attribute_value, +- _("`:' not allowed in encrypted " ++ _("`:' and `\\n' not allowed in encrypted " + "password")); + goto err_value; + } +@@ -1713,19 +1818,14 @@ generic_setpass(struct lu_module *module + } + + /* Now write our changes to the file. */ +- ret = lu_util_field_write(fd, name, field, password, error); ++ ret = lu_util_field_write(e->new_fd, name, field, password, error); + /* Fall through */ + +- err_value: ++err_value: + g_free(value); +-err_lock: +- lu_util_lock_free(lock); +- err_fd: +- close(fd); +-err_fscreate: +- lu_util_fscreate_restore(fscreate); +- err_filename: +- g_free(filename); ++err_editing: ++ ret = editing_close(e, ret, ret, error); /* Commit/rollback happens here. */ ++err_name: + g_free(name); + return ret; + } +@@ -1802,7 +1902,6 @@ lu_files_enumerate(struct lu_module *mod + const char *pattern, struct lu_error **error) + { + int fd; +- gpointer lock; + GValueArray *ret; + GValue value; + char *buf; +@@ -1824,20 +1923,12 @@ lu_files_enumerate(struct lu_module *mod + return NULL; + } + +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- g_free(filename); +- return NULL; +- } +- + /* Wrap the file for stdio operations. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), filename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + g_free(filename); + return NULL; +@@ -1873,7 +1964,6 @@ lu_files_enumerate(struct lu_module *mod + + /* Clean up. */ + g_value_unset(&value); +- lu_util_lock_free(lock); + fclose(fp); + g_free(filename); + +@@ -1902,7 +1992,6 @@ lu_files_users_enumerate_by_group(struct + struct lu_error **error) + { + int fd; +- gpointer lock; + GValueArray *ret; + GValue value; + char *buf, grp[CHUNK_SIZE]; +@@ -1927,21 +2016,12 @@ lu_files_users_enumerate_by_group(struct + return NULL; + } + +- /* Lock the passwd file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- g_free(pwdfilename); +- g_free(grpfilename); +- return NULL; +- } +- + /* Wrap the descriptor in a stdio FILE. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), pwdfilename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + g_free(pwdfilename); + g_free(grpfilename); +@@ -2000,7 +2080,6 @@ lu_files_users_enumerate_by_group(struct + } + /* Close the file. */ + g_value_unset(&value); +- lu_util_lock_free(lock); + fclose(fp); + + /* Open the group file. */ +@@ -2015,22 +2094,12 @@ lu_files_users_enumerate_by_group(struct + return NULL; + } + +- /* Lock the group file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- g_free(pwdfilename); +- g_free(grpfilename); +- g_value_array_free(ret); +- return NULL; +- } +- + /* Wrap the group file in an stdio file. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), grpfilename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + g_free(pwdfilename); + g_free(grpfilename); +@@ -2085,7 +2154,6 @@ lu_files_users_enumerate_by_group(struct + } + + /* Clean up. */ +- lu_util_lock_free(lock); + fclose(fp); + + g_free(pwdfilename); +@@ -2102,7 +2170,6 @@ lu_files_groups_enumerate_by_user(struct + struct lu_error **error) + { + int fd; +- gpointer lock; + GValueArray *ret; + GValue value; + char *buf; +@@ -2126,19 +2193,12 @@ lu_files_groups_enumerate_by_user(struct + goto err_pwdfilename; + } + +- /* Lock it. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- goto err_pwdfilename; +- } +- + /* Open it so that we can use stdio. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), pwdfilename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + goto err_pwdfilename; + } +@@ -2186,7 +2246,6 @@ lu_files_groups_enumerate_by_user(struct + } + g_free(buf); + } +- lu_util_lock_free(lock); + fclose(fp); + + /* Open the groups file. */ +@@ -2198,19 +2257,12 @@ lu_files_groups_enumerate_by_user(struct + goto err_key; + } + +- /* Lock it. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- goto err_key; +- } +- + /* Open it so that we can use stdio. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), grpfilename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + goto err_key; + } +@@ -2267,7 +2319,6 @@ lu_files_groups_enumerate_by_user(struct + g_free(key); + g_value_unset(&value); + +- lu_util_lock_free(lock); + fclose(fp); + g_free(pwdfilename); + g_free(grpfilename); +@@ -2291,7 +2342,6 @@ lu_files_enumerate_full(struct lu_module + struct lu_error **error) + { + int fd; +- gpointer lock; + GPtrArray *ret = NULL; + char *buf; + char *key, *filename; +@@ -2311,19 +2361,12 @@ lu_files_enumerate_full(struct lu_module + goto err_filename; + } + +- /* Lock the file. */ +- if ((lock = lu_util_lock_obtain(fd, error)) == NULL) { +- close(fd); +- goto err_filename; +- } +- + /* Wrap the file up in stdio. */ + fp = fdopen(fd, "r"); + if (fp == NULL) { + lu_error_new(error, lu_error_open, + _("couldn't open `%s': %s"), filename, + strerror(errno)); +- lu_util_lock_free(lock); + close(fd); + goto err_filename; + } +@@ -2358,7 +2401,6 @@ lu_files_enumerate_full(struct lu_module + g_free(key); + } + +- lu_util_lock_free(lock); + fclose(fp); + + err_filename: +diff -up libuser-0.60/tests/files_test.py.CVE-2015-3246 libuser-0.60/tests/files_test.py +--- libuser-0.60/tests/files_test.py.CVE-2015-3246 2013-10-12 23:56:08.000000000 +0200 ++++ libuser-0.60/tests/files_test.py 2015-07-08 15:15:14.061544074 +0200 +@@ -262,6 +262,21 @@ class Tests(unittest.TestCase): + e = self.a.initUser('user6_8') + self.assertRaises(RuntimeError, self.a.addUser, e, False, False) + ++ def testUserAdd9(self): ++ # '\n' in field values ++ # libuser.USERPASSWORD not tested because lu_shadow_user_add_prep() ++ # always replaces it by 'x'. libuser.UIDNUMBER not tested because ++ # ent_has_name_and_id() interprets the value as a number. ++ for field in (libuser.USERNAME, libuser.GIDNUMBER, libuser.GECOS, ++ libuser.HOMEDIRECTORY, libuser.LOGINSHELL, ++ libuser.SHADOWPASSWORD, libuser.SHADOWLASTCHANGE, ++ libuser.SHADOWMIN, libuser.SHADOWMAX, ++ libuser.SHADOWWARNING, libuser.SHADOWINACTIVE, ++ libuser.SHADOWEXPIRE, libuser.SHADOWFLAG): ++ e = self.a.initUser('user_6_9' + field) ++ e[field] = str(e[field][0]) + '\nx' ++ self.assertRaises(RuntimeError, self.a.addUser, e, False, False) ++ + def testUserMod1(self): + # A minimal case + e = self.a.initUser('user7_1') +@@ -421,6 +436,26 @@ class Tests(unittest.TestCase): + e[libuser.USERNAME] = 'user7_7' + self.assertRaises(RuntimeError, self.a.modifyUser, e, False) + ++ def testUserMod8(self): ++ # '\n' in field values ++ # libuser.USERPASSWORD not tested because lu_shadow_user_add_prep() ++ # always replaces it by 'x'. libuser.UIDNUMBER not tested because ++ # ent_has_name_and_id() interprets the value as a number. ++ for field in (libuser.USERNAME, libuser.USERPASSWORD, libuser.GIDNUMBER, ++ libuser.GECOS, libuser.HOMEDIRECTORY, libuser.LOGINSHELL, ++ libuser.SHADOWPASSWORD, libuser.SHADOWLASTCHANGE, ++ libuser.SHADOWMIN, libuser.SHADOWMAX, ++ libuser.SHADOWWARNING, libuser.SHADOWINACTIVE, ++ libuser.SHADOWEXPIRE, libuser.SHADOWFLAG): ++ e = self.a.initUser('user7_9' + field) ++ self.a.addUser(e, False, False) ++ del e ++ e = self.a.lookupUserByName('user7_9' + field) ++ self.assertIsNotNone(e) ++ self.assertNotIn('\n', str(e[field][0])) ++ e[field] = str(e[field][0]) + '\nx' ++ self.assertRaises(RuntimeError, self.a.modifyUser, e, False) ++ + def testUserDel(self): + e = self.a.initUser('user8_1') + self.a.addUser(e, False, False) +@@ -619,6 +654,22 @@ class Tests(unittest.TestCase): + crypted = crypt.crypt('a:b', e[libuser.SHADOWPASSWORD][0]) + self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted]) + ++ def testUserSetpass5(self): ++ # '\n' in field value ++ e = self.a.initUser('user12_5') ++ self.a.addUser(e, False, False) ++ del e ++ e = self.a.lookupUserByName('user12_5') ++ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0]) ++ self.assertRaises(SystemError, self.a.setpassUser, e, 'a\nb', True) ++ self.a.setpassUser(e, 'a\nb', False) ++ del e ++ e = self.a.lookupUserByName('user12_5') ++ self.assertEqual(e[libuser.SHADOWPASSWORD][0][:3], '$1$') ++ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0]) ++ crypted = crypt.crypt('a\nb', e[libuser.SHADOWPASSWORD][0]) ++ self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted]) ++ + def testUserRemovepass(self): + e = self.a.initUser('user13_1') + e[libuser.SHADOWPASSWORD] = '03dgZm5nZvqOc' +@@ -884,6 +935,20 @@ class Tests(unittest.TestCase): + e = self.a.initGroup('group21_5') + self.assertRaises(RuntimeError, self.a.addGroup, e) + ++ def testGroupAdd6(self): ++ # '\n' in field values ++ # libuser.GROUPPASSWORD not tested because lu_shadow_group_add_prep() ++ # always replaces it by 'x'. libuser.GIDNUMBER not tested because ++ # ent_has_name_and_id() interprets the value as a number. ++ for field in (libuser.GROUPNAME, libuser.SHADOWPASSWORD, ++ libuser.MEMBERNAME, libuser.ADMINISTRATORNAME): ++ e = self.a.initGroup('group_21_6' + field) ++ if e.has_key(field): ++ e[field] = str(e[field][0]) + '\nx' ++ else: ++ e[field] = field + '\nx' ++ self.assertRaises(RuntimeError, self.a.addGroup, e) ++ + def testGroupMod1(self): + # A minimal case + e = self.a.initGroup('group22_1') +@@ -997,6 +1062,25 @@ class Tests(unittest.TestCase): + e[libuser.GROUPNAME] = 'group22_6' + self.assertRaises(RuntimeError, self.a.modifyGroup, e) + ++ def testGroupMod7(self): ++ # '\n' in field values ++ # libuser.GIDNUMBER not tested because ent_has_name_and_id() interprets ++ # the value as a number. ++ for field in (libuser.GROUPNAME, libuser.GROUPPASSWORD, ++ libuser.SHADOWPASSWORD, libuser.MEMBERNAME, ++ libuser.ADMINISTRATORNAME): ++ e = self.a.initGroup('group22_8' + field) ++ self.a.addGroup(e) ++ del e ++ e = self.a.lookupGroupByName('group22_8' + field) ++ self.assertIsNotNone(e) ++ if e.has_key(field): ++ self.assertNotIn('\n', str(e[field][0])) ++ e[field] = str(e[field][0]) + '\nx' ++ else: ++ e[field] = field + '\nx' ++ self.assertRaises(RuntimeError, self.a.modifyGroup, e) ++ + def testGroupDel(self): + e = self.a.initGroup('group23_1') + self.a.addGroup(e) +@@ -1190,6 +1274,22 @@ class Tests(unittest.TestCase): + crypted = crypt.crypt('a:b', e[libuser.SHADOWPASSWORD][0]) + self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted]) + ++ def testGroupSetpass4(self): ++ # '\n' in field value ++ e = self.a.initGroup('group27_5') ++ self.a.addGroup(e) ++ del e ++ e = self.a.lookupGroupByName('group27_5') ++ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0]) ++ self.assertRaises(SystemError, self.a.setpassGroup, e, 'a\nb', True) ++ self.a.setpassGroup(e, 'a\nb', False) ++ del e ++ e = self.a.lookupGroupByName('group27_5') ++ self.assertEqual(e[libuser.SHADOWPASSWORD][0][:3], '$1$') ++ self.assertNotIn('\n', e[libuser.SHADOWPASSWORD][0]) ++ crypted = crypt.crypt('a\nb', e[libuser.SHADOWPASSWORD][0]) ++ self.assertEqual(e[libuser.SHADOWPASSWORD], [crypted]) ++ + def testGroupRemovepass(self): + e = self.a.initGroup('group28_1') + e[libuser.SHADOWPASSWORD] = '07Js7N.eEhbgs' diff --git a/SPECS/libuser.spec b/SPECS/libuser.spec index 80244f9..b45cf44 100644 --- a/SPECS/libuser.spec +++ b/SPECS/libuser.spec @@ -2,11 +2,12 @@ Name: libuser Version: 0.60 -Release: 5%{?dist} +Release: 7%{?dist} Group: System Environment/Base License: LGPLv2+ URL: https://fedorahosted.org/libuser/ Source: https://fedorahosted.org/releases/l/i/libuser/libuser-%{version}.tar.xz +Patch0: libuser-CVE-2015-3246.patch BuildRequires: glib2-devel, linuxdoc-tools, pam-devel, popt-devel, python2-devel BuildRequires: cyrus-sasl-devel, libselinux-devel, openldap-devel # To make sure the configure script can find it @@ -46,6 +47,8 @@ administering user and group accounts. %prep %setup -q +%patch0 -p1 -b .CVE-2015-3246 + %build %configure --with-selinux --with-ldap --with-html-dir=%{_datadir}/gtk-doc/html make @@ -96,6 +99,14 @@ python -c "import libuser" %{_datadir}/gtk-doc/html/* %changelog +* Wed Jul 8 2015 Miloslav Trmač - 0.60-7 +- Update CVE-2015-3246 patch based on review comments + Resolves: #1235519 + +* Fri Jun 26 2015 Miloslav Trmač - 0.60-6 +- Fix CVE-2015-3246 + Resolves: #1235519 + * Fri Jan 24 2014 Daniel Mach - 0.60-5 - Mass rebuild 2014-01-24