Search code examples
cfopenfortifypath-manipulation

Fortify : Path Manipulation in C - White List Implementation doesn't work - fopen issue


Hello everyone I have a fortify issue "Path manipulation" it produced by fopen use. According with fortify I could implement a white list in order to fix it, so there have my white list validator:

white_list.c

#define BUFF_WHITE_LIST_FILE 200
const char *white_list_validator( char *variable )
{
  FILE *fp = NULL;
  ssize_t read;
  char * line = NULL;
  size_t len = 0;
  char white_list_file_buff[BUFF_WHITE_LIST_FILE];
  if ( __secure_getenv("WHITE_LIST_FILE") == NULL )
    return NULL;
  else
  {
    strncpy(white_list_file_buff,
         __secure_getenv("WHITE_LIST_FILE"),sizeof(white_list_file_buff)-1);
    fp = fopen(white_list_file_buff,"r");
    if ( fp == NULL )
        return NULL;
    else
    {
        while( (read = getline(&line, &len, fp)) != -1 )
        {
            if ( strncmp(line,variable,read - 1) == 0 ){
                fclose(fp);
                return variable;
            }

        }
        fclose(fp);

    }
    if(line)
        free(line);
 }
 return NULL;
}

it return NULL if it doesn't find the variable inside White.list (*) or return a pointer to char if it find it

int main( int argc, char **argv ) {

    FILE *fp = NULL;
    char mrd[50]={0};
    const char *ptr = white_list_validator(argv[1]);

    if ( argv[1] == NULL )
        return -1;

    if(ptr==NULL)
        return -1;
    else
    {
        strncpy(mrd,ptr,sizeof(mrd)-1);
        printf("variables found : %s\n",mrd);

        fp = fopen(mrd,"w");  <------   SINK
        if ( fp == NULL ){
                printf("line 22\n");
                exit(1);
        }
        fprintf(fp, "%s %s %s %d", "We", "are", "in", 2077);
        fclose(fp);

    }

  return 0;
 }

but when I run the fortify report appear a manipulation path vulnerability in fopen, I do not know why .You can see in the code, before that manage to file with fopen it validated for a white_list_validator. so anybody has an idea why it does not work correctly?

NOTE(*) : export WHITE_LIST_FILE=/path/White.list

cat White.list

test1

test2

something

When I run the binary:

./white_list something

variables found : something


Solution

  • A quick search of __secure_getenv lead to this page:

    https://refspecs.linuxfoundation.org/LSB_1.1.0/gLSB/baselib---secure-getenv-1.html

    Quote:

    __secure_getenv(name) has the same specification as getenv(name) with the exception that if the program is running SUID or SGID enabled, the result is always NULL.

    So, the question is: is your program running with set SUID or SGID bits? As far as I can see, __secure_getenv has been renamed to secure_getenv (my man page says it appeared in glibc 2.17). You should use that instead.

    Another cause could be: if the length of the source string is longer than the size argument of strncpy, it won't add the '\0' terminating byte. When using strncpy you should always make sure to write the '\0' terminating byte.

    man strcpy

    #include <string.h>
    char *strncpy(char *dest, const char *src, size_t n);
    

    The strncpy() function is similar, except that at most n bytes of src are copied. Warning: If there is no null byte among the first n bytes of src, the string placed in dest will not be null-terminated.

    white_list_file_buff might not be '\0'-terminated, thus fopen fails. But you say you did export WHITE_LIST_FILE=/path/White.list. Is /path/White.list the real value you used or some path that is longer than 200 characters?

    Also your code here

    while( (read = getline(&line, &len, fp)) != -1 )
    {
        else if ( strncmp(line,variable,read - 1) == 0 ){
            fclose(fp);
            return variable;
        }
    
    }
    

    is either wrong or did you forget to paste the whole code? There is no previous if for that else.

    Assuming that you've made a copy&paste error, how is the format if White.list? Every line contains nothing but the name of variables? Your strncmp compare the whole line you know, and if you want to match only a substring, you should use strstr.