View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0009307 | GNUnet | util library | public | 2024-11-01 13:05 | 2024-12-09 18:03 |
| Reporter | fefe | Assigned To | Christian Grothoff | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | no change required | ||
| Target Version | 0.23.0 | Fixed in Version | 0.23.0 | ||
| Summary | 0009307: GNUNET_DISK_mktemp: race condition | ||||
| Description | GNUNET_DISK_mktemp calls mkstemp, which is good. That is the way to do it. It avoids the file system race by giving you a descriptor to an open file. However, GNUNET_DISK_mktemp then closes that file and returns the file name, which opens the race again that mkstemp so artfully avoided! If your API returns the file name and not the descriptor, then your API is insecure and can never be implemented or called safely. | ||||
| Tags | No tags attached. | ||||
|
|
I think this needs some discussion. Our use of GNUNET_DISK_mktemp usually does not really require us to use exactly that file. We kind of use it to 1. Create a temporary file name 2. Make sure that a file with that name exists I understand the problem. But I am torn. Should we really deprecate all APIs that take a filename to load a file that may potentially have been created by GNUNET_DISK_mktemp beforehand? Or should we just rename GNUNET_DISK_mktemp to something less triggery? Its use does not seem to imply "temporary" with respect to the file created. I will leave this open for now. |
|
|
I don't quite understand the problem. After mktemp, the file that was created is *owned* by our UID. So no _other_ UID can go in there and create/steal our filename (ok, except root, but that's beyond the point). Just test this: $ touch /tmp/foo $ sudo -u nobody rm /tmp/foo # => fails (ditto for mv) The original race problem with mktemp() was NOT primarily fixed by returning the FD (that's more of a convenience and I guess helps against the owner accidentally deleting the file before using it), but by actually *creating* the file. The vanilla mktemp() is racy because it only checks for non-existence, but then didn't actually create the file, giving another process the chance of creating it before the application could create it. The fact that it ALSO returns the 'fd' is largely immaterial here, unless you care about races by processes with the same UID, which we really do not. Note that the fstat-races are different, as here the file may not be owned by the same user. So overall, my feeling is that the report is actually wrong, because there is no race condition here. |
|
|
Released |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2024-11-01 13:05 | fefe | New Issue | |
| 2024-11-03 20:34 | schanzen | Target Version | => 0.22.3 |
| 2024-11-03 20:59 | schanzen | Note Added: 0023643 | |
| 2024-11-03 20:59 | schanzen | Assigned To | => Christian Grothoff |
| 2024-11-03 20:59 | schanzen | Status | new => assigned |
| 2024-11-04 07:46 | Christian Grothoff | Note Added: 0023648 | |
| 2024-11-04 07:46 | Christian Grothoff | Status | assigned => feedback |
| 2024-11-07 11:47 | schanzen | Status | feedback => resolved |
| 2024-11-07 11:47 | schanzen | Resolution | open => no change required |
| 2024-11-14 09:43 | schanzen | Target Version | 0.22.3 => 0.23.0 |
| 2024-12-09 18:03 | schanzen | Fixed in Version | => 0.23.0 |
| 2024-12-09 18:03 | schanzen | Note Added: 0023833 | |
| 2024-12-09 18:03 | schanzen | Status | resolved => closed |
