question about mktemp security warning

I see a security warning in the analyzer that saids that using mktemp is always wrong.
I though using mktemp + open(path, O_CREAT | O_EXCL) was safe.

Did I miss something, or is the analyzer a little too much strict about this function ?

I known that mkstemp should be preferred, but some API (like SQLite) provide only an "open" like function, and no "fdopen" like function, so mkstemp is useless in these case.

-- Jean-Daniel

I see a security warning in the analyzer that saids that using mktemp is always wrong.
I though using mktemp + open(path, O_CREAT | O_EXCL) was safe.

Did I miss something, or is the analyzer a little too much strict about this function ?

This is moderately safe, but you have to make sure that you check the open() doesn't fail. You end up reimplementing mkstemp(), which is pretty much guaranteed to be the wrong thing to do in any situation.

I known that mkstemp should be preferred, but some API (like SQLite) provide only an "open" like function, and no "fdopen" like function, so mkstemp is useless in these case.

mkstemp isn't useless in these cases. You can use mkstemp like this:

char *name = strdup("templateXXX");
close(mkstemp(name));
custom_open_function(name);

If you are doing two-stage creation with a more complex API, you should use mkdtemp() and then create the temporary file inside the newly created temporary directory.

Note that the correct way of creating a temporary database with sqlite is to pass the empty string to sqlite3_open() and not use mk[s]temp() at all. This will also delete the database for you automatically when you close it.

David

-- Send from my Jacquard Loom

The problem with mktemp is a possible race condition.
Imagine that 2 programs call mktemp(). It may happen that they both get the same file name (because they didn't create the file yet), and in the end each program will have a filename that is not unique.

Nuno

I see a security warning in the analyzer that saids that using mktemp is always wrong.

I though using mktemp + open(path, O_CREAT | O_EXCL) was safe.

Did I miss something, or is the analyzer a little too much strict about this function ?

This is moderately safe, but you have to make sure that you check the open() doesn’t fail. You end up reimplementing mkstemp(), which is pretty much guaranteed to be the wrong thing to do in any situation.

I known that mkstemp should be preferred, but some API (like SQLite) provide only an “open” like function, and no “fdopen” like function, so mkstemp is useless in these case.

mkstemp isn’t useless in these cases. You can use mkstemp like this:

char *name = strdup(“templateXXX”);
close(mkstemp(name));
custom_open_function(name);

Thanks, I will try this.

If you are doing two-stage creation with a more complex API, you should use mkdtemp() and then create the temporary file inside the newly created temporary directory.

Note that the correct way of creating a temporary database with sqlite is to pass the empty string to sqlite3_open() and not use mk[s]temp() at all. This will also delete the database for you automatically when you close it.

Not an option in my case.
I’m using sqlite as a document file format, and I want to implement a “safe save as” function that do not override an existing file if it failed to save the document.
So I’m creating a temporary base, I populate it and close it, and then I use exchangedata(2) (atomically exchange data between two files on Darwin) to replace the existing file.
So I must have control over which file system the file is created (it must be the same than the target file), and also, I don’t want that the file be deleted on close.

David

– Jean-Daniel