Skip to content

lib: multiple issues in update_password #603

Description

@xlzs0

lib/password.c, lines against 59b041d

L133: fopen truncating files on open
L156: inverted condition never matching entries
L171: off by one in field replacement
L172: xmprintf padding instead of truncating
L177: use after free when out==line, double free when out==oo, use after free on caller side when out==entry
L186: found not incremented on append

proposed patch:

--- a/lib/password.c
+++ b/lib/password.c
@@ -130,7 +130,7 @@ int update_password(char *filename, char *username, char *entry, int pos)
   long long ll = 0;

   // Open old filename ("r" won't let us lock) and get blocking lock
-  if (!(ofp = fopen(filename, "w+")) || 0>fcntl(fileno(ofp), F_SETLK, &lock)\
+  if (!(ofp = fopen(filename, "r+")) || 0>fcntl(fileno(ofp), F_SETLK, &lock)\
       || fstat(fileno(ofp), &st))
   {
     perror_msg_raw(filename);
@@ -153,7 +153,7 @@ int update_password(char *filename, char *username, char *entry, int pos)
     // find matching line
     oo = 0;
     start = end = chomp(line);
-    if (strncmp(line, username, len) || !(line[len] && line[len]!=':'))
+    if (strncmp(line, username, len) || line[len] != ':')
       out = line;
     else {
       found++;
@@ -168,13 +168,12 @@ int update_password(char *filename, char *username, char *entry, int pos)
           while (*end && *end != ':') end++;
           if (!pos || !*end) break;
         }
-        if (pos>=0) out = line;
-        else oo = out = xmprintf("%*s%s%s\n", (int)(start-line),line,entry,end);
+        if (pos>0) out = line;
+        else oo = out = xmprintf("%.*s%s%s\n", (int)(start-line),line,entry,end);
       }
     }
     if (out) {
       rc = dprintf(nfd, "%s\n", out);
-      free(out);
       if (rc<0) {
         perror_msg_raw(ff);
         goto free_storage;
@@ -183,7 +182,10 @@ int update_password(char *filename, char *username, char *entry, int pos)
     }
   }
   free(line);
-  if (!found && entry && strchr(entry, ':')) dprintf(nfd, "%s\n", entry);
+  if (!found && entry && strchr(entry, ':')) {
+    dprintf(nfd, "%s\n", entry);
+    found++;
+  }
   fsync(nfd);
   close(nfd);  // automatically unlocks

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions