Loading HuntDB...

CVE-2022-32207: Unpreserved file permissions

Medium
C
curl
Submitted None
Reported by nyymi

Vulnerability Details

Technical details and impact analysis

Business Logic Errors
## 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

Report Details

Additional information and metadata

State

Closed

Substate

Resolved

Submitted

Weakness

Business Logic Errors