CVE-2022-32207: Unpreserved file permissions

How it was found

Summary:

Curl fails to preserve file permissions when writing:

• CURLOPT_COOKIEJAR database
• CURLOPT_ALTSVC database
• CURLOPT_HSTS database

Instead the permissions is always reset to 0666 & ~umask if the file is updated.

As a result a file that was before protected against read access by other users becomes other user readable (as long as umask doesn't have bit 2 set). Out of these files only the CURLOPT_COOKIEJAR is likely to contain sensitive information.

In addition curl will replace softlink to the database with locally written database, or if the application is run privileged, specifying "/dev/null" as a file name can lead to system overwriting the special file and result in inoperable system.

This is CWE-281: Improper Preservation of Permissions

Steps To Reproduce:

1. umask 022
2. install -m 600 /dev/null cookie.db
3. curl -b cookie.db -c cookie.db https://google.com
4. ls -l cookie.db

At least for CURLOPT_COOKIEJAR this vulnerability was introduced in https://github.com/curl/curl/commit/b834890a3fa3f525cd8ef4e99554cdb4558d7e1b - this change was introduced to fix a issue https://github.com/curl/curl/issues/4914

Fix recommendations

If a file file is created and moved over a the old one, only do this if the file is regular file. Anything else is likely going to end up causing unexpected behaviour, outright failing, or if the user has high enough permissions, damage to the operating system.

Safe cloning of file permissions can only be achieved if the owner / group of the file match the current user (else group permissions might be incorrect). Hence creating a new file and moving it over the old one should IMO only be attempted if the file user and group match that of the previous file.

If a method of creating a new file is still desired, something like this could be attempted to cover the most use cases:

 /* If old file is a regular file attempt creating a new file with same ownership */
 struct stat st;
 if (stat(filename, &st) != -1 && S_ISREG(st.st_mode)) {
   FILE *file;
   int fd;
   struct stat nst;
   fd = open(tempstore, O_CREAT | O_EXCL, 0700);
   if (fd == -1)
     goto fail;
   if (fstat(fd, &nst) == -1 ||
      nst.st_uid != st.st_uid || nst.st_gid != st.st_gid) {
     /* newly created file doesn't have same ownership, we can't proceed safely */
     close(fd);
     unlink(tempstore);
     goto fail; // or perhaps try direct write instead?
    }
   /* use same mode as old file */
   if (fchmod(fd, st.st_mode) == -1) {
     close(fd);
     unlink(tempstore);
     goto fail;
   }
   file = fdopen(fd, FOPEN_WRITETEXT);
   if (!file) {
     close(fd);
     unlink(tempstore);
     goto fail;
  }
  /* write to file */
  /* if successful move file over filename etc */
 }
 else  {
   /* use direct file write */
 }

Impact

Leak of sensitive information