[server] comment additions regarding Coverity low priority TOCTOU issues

This commit is contained in:
Michael Rash
2013-06-04 22:17:59 -04:00
parent 59eb7fcf0f
commit 17974a1c05
2 changed files with 18 additions and 2 deletions
+14
View File
@@ -935,6 +935,20 @@ parse_access_file(fko_srv_options_t *opts)
verify_file_perms_ownership(opts->config[CONF_ACCESS_FILE]);
/* A note on security here: Coverity flags the following fopen() as a
* Time of check time of use (TOCTOU) bug with a low priority due to the
* previous stat() call above. I.e., the access.conf file on disk could
* have been changed between the stat() and the fopen() causing a TOCTOU
* bug. While technically this is true, the return value of fopen() is
* also checked below so stat() success does not imply we assume fopen()
* success. Also, we could just remove the stat() and
* verify_file_perms_ownership() calls above to "fix" the bug, but this
* would actually make things easier for an attacker that has already
* compromised the local system since access.conf could be changed to, say,
* a symbolic link (for which verify_file_perms_ownership() throws a
* warning), and then there is no race at all before the fopen(). I.e.
* forcing an attacker to do the race makes things harder for them.
*/
if ((file_ptr = fopen(opts->config[CONF_ACCESS_FILE], "r")) == NULL)
{
fprintf(stderr, "[*] Could not open access file: %s\n",
+4 -2
View File
@@ -206,8 +206,7 @@ parse_config_file(fko_srv_options_t *opts, const char *config_file)
struct stat st;
/* First see if the config file exists. If it doesn't, complain
* and go on with program defaults.
/* Make sure the config file exists.
*/
if(stat(config_file, &st) != 0)
{
@@ -218,6 +217,9 @@ parse_config_file(fko_srv_options_t *opts, const char *config_file)
verify_file_perms_ownership(config_file);
/* See the comment in the parse_access_file() function regarding security
* here relative to a TOCTOU bug flagged by Coverity.
*/
if ((cfile_ptr = fopen(config_file, "r")) == NULL)
{
fprintf(stderr, "[*] Could not open config file: %s\n",