DeforaNetworks/fwknop
DeforaNetworks/fwknop
3
0
Fork 0
Code Releases Activity

Avoid TOCTOU by using fstat() after open()

Browse Source 
This also needs fileno() after fopen(). This time it addresses the code
for the client.

This is the third part of three for Coverity issue 1355235.
This commit is contained in:
Pierre Pronchery 2018-08-27 20:00:02 +02:00
parent cb8632f4db
commit 2def3bb0e8
5 changed files with 18 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
client/config_init.c
View File

@ -696,7 +696,7 @@ set_rc_file(char *rcfile, fko_cli_options_t *options)
* client consumes a proper rc file with strict permissions set (thanks
* to Fernando Arnaboldi from IOActive for pointing this out).
*/
if(verify_file_perms_ownership(rcfile) != 1)
if(verify_file_perms_ownership(rcfile, -1) != 1)
exit(EXIT_FAILURE);
return;

17
client/fwknop.c
View File

@ -893,15 +893,18 @@ show_last_command(const char * const args_save_file)
char args_str[MAX_LINE_LEN] = {0};
FILE *args_file_ptr = NULL;
if(verify_file_perms_ownership(args_save_file) != 1)
return 0;
if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) {
log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
args_save_file);
return 0;
}
if(verify_file_perms_ownership(args_save_file, fileno(args_file_ptr)) != 1)
{
fclose(args_file_ptr);
return 0;
}
if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) {
log_msg(LOG_VERBOSITY_NORMAL,
"Last fwknop client command line: %s", args_str);
@ -928,15 +931,17 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
memset(argv_new, 0x0, sizeof(argv_new));
if(verify_file_perms_ownership(args_save_file) != 1)
return 0;
if ((args_file_ptr = fopen(args_save_file, "r")) == NULL)
{
log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
args_save_file);
return 0;
}
if(verify_file_perms_ownership(args_save_file, fileno(args_file_ptr)) != 1)
{
fclose(args_file_ptr);
return 0;
}
if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL)
{
args_str[MAX_LINE_LEN-1] = '\0';

6
client/utils.c
View File

@ -56,17 +56,17 @@ static fko_protocol_t fko_protocol_array[] =
};
int
verify_file_perms_ownership(const char *file)
verify_file_perms_ownership(const char *file, int fd)
{
int res = 1;
#if HAVE_STAT
#if HAVE_FSTAT && HAVE_STAT
struct stat st;
/* Every file that the fwknop client deals with should be owned
* by the user and permissions set to 600 (user read/write)
*/
if((stat(file, &st)) == 0)
if((fd >= 0 && fstat(fd, &st) == 0) || stat(file, &st) == 0)
{
/* Make sure it is a regular file
*/

2
client/utils.h
View File

@ -52,7 +52,7 @@
/* Prototypes
*/
int verify_file_perms_ownership(const char *file);
int verify_file_perms_ownership(const char *file, int fd);
int resolve_dst_addr(const char *dns_str, struct addrinfo *hints,
char *ip_str, size_t ip_bufsize, fko_cli_options_t *opts);
short proto_inttostr(int proto, char *proto_str, size_t proto_size);

4
server/utils.c
View File

@ -111,13 +111,13 @@ is_valid_file(const char *path)
int
verify_file_perms_ownership(const char *file, int fd)
{
#if HAVE_FSTAT
#if HAVE_FSTAT && HAVE_STAT
struct stat st;
/* Every file that fwknopd deals with should be owned
* by the user and permissions set to 600 (user read/write)
*/
if(fstat(fd, &st) == 0)
if((fd >= 0 && fstat(fd, &st) == 0) || stat(file, &st) == 0)
{
/* Make sure it is a regular file
*/