Need help following rule logic with secondary locations

Sonar 8.4.

I have the following code and sonar is reporting a possible out of bounds memory access (which may very well be a legitimate issue) but I’m having trouble following sonar’s branch analysis.

Also, I am unable to post in the ‘Get Help’ section, that’s why I’m posting here.

#include <stdio.h>                                                                                                                                                                                                                            
#include <stdlib.h>                                                                                                                                                                                                                           
#include <string.h>                                                                                                                                                                                                                           
                                                                                                                                                                                                                                              
//#include "mylib.h"                                                                                                                                                                                                                          
                                                                                                                                                                                                                                              
char **strtok_multi(char *src, char *delimiter, int *num_strings)                                                                                                                                                                             
{                                                                                                                                                                                                                                             
    register int i = 0;                                                                                                                                                                                                                       
    register int j = 0;                                                                                                                                                                                                                       
    int len = 0;                                                                                                                                                                                                                              
    int num_str = 0;                                                                                                                                                                                                                          
    char **str_array = NULL;                                                                                                                                                                                                                  
    char *ptr = NULL;                                                                                                                                                                                                                         
    char *buf = NULL;                                                                                                                                                                                                                         
                                                                                                                                                                                                                                              
    len = strlen(src);                                                                                                                                                                                                                        
                                                                                                                                                                                                                                              
    // make a copy of the src string                                                                                                                                                                                                          
    buf = strdup(src);                                                                                                                                                                                                                        
    if (!buf) return NULL;                                                                                                                                                                                                                    
                                                                                                                                                                                                                                              
    // count occurences of delimiter, plus the final string if necessary                                                                                                                                                                      
    for (i=0; i<len; i++) {                                                                                                                                                                                                                   
        if (strchr(delimiter, buf[i])) {                                                                                                                                                                                                      
            num_str++;                                                                                                                                                                                                                        
        }                                                                                                                                                                                                                                     
        if (i+1 == len) {                                                                                                                                                                                                                     
            num_str++;                                                                                                                                                                                                                        
        }                                                                                                                                                                                                                                     
    }                                                                                                                                                                                                                                         
//    num_str++;                                                                                                                                                                                                                              
                                                                                                                                                                                                                                              
    // make an array of strings                                                                                                                                                                                                               
    str_array = (char **)malloc((num_str+1)*(sizeof(char *)));                                                                                                                                                                                
    if (!str_array) { free(buf); return NULL; }                                                                                                                                                                                               
                                                                                                                                                                                                                                              
    // null terminate the str_array for good measure                                                                                                                                                                                          
    str_array[num_str] = NULL;                                                                                                                                                                                                                
                                                                                                                                                                                                                                              
    if (num_str == 0) {                                                                                                                                                                                                                       
                                                                                                                                                                                                                                              
        // for an empty string, there will be no entries in the array,                                                                                                                                                                        
        // so when we go to clean up in mie_strtok_free, there will                                                                                                                                                                           
        // be no reference to buf (which was malloced), so we need to                                                                                                                                                                         
        // free it here.                                                                                                                                                                                                                      
        free(buf);                                                                                                                                                                                                                            
                                                                                                                                                                                                                                              
    } else {                                                                                                                                                                                                                                  
        ptr = buf;                                                                                                                                                                                                                            
        for (i=0; i<len; i++) {                                                                                                                                                                                                               
            if (strchr(delimiter, buf[i])) {                                                                                                                                                                                                  
                buf[i] = 0; // null terminate every string                                                                                                                                                                                    
                str_array[j] = ptr;                                                                                                                                                                                                           
                j++;                                                                                                                                                                                                                          
                ptr = buf+i+1;                                                                                                                                                                                                                
            }                                                                                                                                                                                                                                 
                                                                                                                                                                                                                                              
            if (i+1 == len) {                                                                                                                                                                                                                 
                str_array[j++] = ptr;                                                                                                                                                                                                         
            }
        }                                                                                                                                                                                                                                     
    }                                                                                                                                                                                                                                         
                                                                                                                                                                                                                                              
    if (num_strings) *num_strings = num_str;                                                                                                                                                                                                  
                                                                                                                                                                                                                                              
    return str_array;                                                                                                                                                                                                                         
}                                                                                                                                                                                                                                             
                                                                                                                                                                                                                                              
int main(int argc, char *argv[])                                                                                                                                                                                                              
{                                                                                                                                                                                                                                             
    char *withoutDelim = "HelloThereWorld";                                                                                                                                                                                                   
    char *withDelim = "Hello,There,World";                                                                                                                                                                                                    
    char **tmp = NULL;                                                                                                                                                                                                                        
    int num = 0, x = 0;                                                                                                                                                                                                                       
                                                                                                                                                                                                                                              
    tmp = strtok_multi(withoutDelim, ",:", &num);                                                                                                                                                                                             
    printf("\nFound %d str sections\n", num);                                                                                                                                                                                                 
    for (x = 0; x < num; x++) {                                                                                                                                                                                                               
        printf("Section %d: %s\n", num, tmp[x]);                                                                                                                                                                                              
    }                                                                                                                                                                                                                                         
    free(tmp);                                                                                                                                                                                                                                
    tmp = strtok_multi(withDelim, ",:", &num);                                                                                                                                                                                                
    printf("\nFound %d str sections\n", num);                                                                                                                                                                                                 
    for (x = 0; x < num; x++) {                                                                                                                                                                                                               
        printf("Section %d: %s\n", num, tmp[x]);                                                                                                                                                                                              
    }                                                                                                                                                                                                                                         
    free(tmp);                                                                                                                                                                                                                                
} 

Ignoring any possible issues with the code, I’m just trying to resolve the following questions:

  • In the first for loop of the strtok_multi function sonar says it hits the strchr function three times evaluating to false.
    • In the second for loop in the same function, it again hits the strchr function three times, but the last time evaluates to true this time. Is this even possible? Both variables buf and delimiter are unchanged during the execution of this function. Shouldn’t both loops execute the same number of times with the same evaluations? Or does sonar not track state this way inside of a function?
  • I realize it can’t apply to all scenarios, but is there any way I can get information about what data sonar is using to execute this code path? I would love to see the strings sonar is assuming to arrive at the given branches.

Hello @KaibutsuX,

Thank you for reporting this situation, I’ve been able to reproduce it on my side. To implement advanced rules such as buffer overflows that require to model the actual execution of the code, we rely on the Clang Static Analyzer engine. You can find more advanced details on how this works in the documentation of this tool.

While this tool can find very interesting bugs, it is also susceptible to false positives (Rice theorem even proves that it’s not possible to be perfect for such rules). And it currently is not very explicit about how it reaches some conclusion. We are aware of this issue, which is clearly something we would like to improve.

To give more accurate answers to your questions:

  • In the first for loop of the strtok_multi function sonar says it hits the strchr function three times evaluating to false: I’d like to add a small nuance: it just reports that in one of the paths that led to finding a buffer overflow, the first loop was executed 3 times. It does not mean that the loop actually is evaluated 3 times. The function is analyzed in isolation, and we don’t know what are going to be its inputs.
  • but the last time evaluates to true this time. Is this even possible? The analysis does not know the behavior of the strchr function, and is just assumes that is can return anything at any time, even when it’s argument are unchanged (it could have internal state)
  • I realize it can’t apply to all scenarios, but is there any way I can get information about what data sonar is using to execute this code path? Not directly from our tool. But if you really are motivated, you can use the debugging tools available with Clang Static Analyzer to reproduce this outside of SonarSource environment. Unfortunately, it requires quite some expertise to do so. I tried to dig a little bit in your code to see if the debugging tools would provide me more information, but up to now,I did not notice anything of interest.

I hope this answers your questions.

2 Likes