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

Question

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

Answer 1

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.

Answer 2

Your whitelist approach approach obtains the name of the whitelist from an environment variable. This is so easily manipulable as to not even justify the added complexity of implementing a whitelist in the first place, and it certainly should be cause for its own path-manipulation vulnerability. Using __secure_getenv() to obtain the value doesn't really help; it just means that your program won't work if run in a secure context.

Even if the whitelist's own path were fixed at compile time, you're still obtaining the whitelisted paths from an external source. That's harder to manipulate than totally unverified user input, but it doesn't really resolve the vulnerability.

If you cannot compile your whitelist into the program, then consider instead hardcoding a whitelist of path prefixes and a whitelist of characters that may appear in the remaining path components. It's unclear whether Fortify would actually recognize this as resolving the issue, but that would be sufficient for me to be satisfied to set the issue's analysis to "Not an issue".

Answer 3

Fortify generally will not recognize whitelist validation as fixing the issue - once you've verified the routine, you need to teach it that this is a validation routine. This will ensure any trace that correctly validates with this routine is not reported.

You should ensure you've addressed the other comments, and validated the underlying routine itself is secure. Then you should register the whitelist routine as a validation routine by adding a custom rule via the rules editor. Best way to do this is a validation / cleansing rule that adds a taint flag of taintFlag="VALIDATED_PATH_MANIPULATION" - the sink rules for path manipulation should not report issues with this taint.

For more information on using the taint flags see https://community.softwaregrp.com/dcvta86296/attachments/dcvta86296/fortify-discussions/2950/1/HP_Fortify_SCA_Custom_Rules_Guide_4.21.pdf