Problem 1: tmp_config_str is 256 chars max. If attacker manages to make openblas_config_str + gotoblas_config_str + tmpstr to be more than or equal to 256 chars (e.g. by manipulating VERSION attribute), it opens a window for a potential buffer overflow attack. For now it's not the case, max length is obviously less than 265 chars.
Problem 2: static code checks of potential lib consumers cannot analyze the size of the openblas_config_str and may report false positives about buffer overflow. It makes devs triage it for each fork.
Suggested fix: it would be great to change strcpy to strncpy and strcat to strncat.
|
char* CNAME(void) { |
|
char tmpstr[20]; |
|
strcpy(tmp_config_str, openblas_config_str); |
|
#ifdef DYNAMIC_ARCH |
|
strcat(tmp_config_str, gotoblas_corename()); |
|
#endif |
|
if (openblas_get_parallel() == 0) |
|
sprintf(tmpstr, " SINGLE_THREADED"); |
|
else |
|
snprintf(tmpstr,19," MAX_THREADS=%d",MAX_CPU_NUMBER); |
|
strcat(tmp_config_str, tmpstr); |
|
return tmp_config_str; |
|
} |
Problem 1:
tmp_config_stris 256 chars max. If attacker manages to makeopenblas_config_str+gotoblas_config_str+tmpstrto be more than or equal to 256 chars (e.g. by manipulating VERSION attribute), it opens a window for a potential buffer overflow attack. For now it's not the case, max length is obviously less than 265 chars.Problem 2: static code checks of potential lib consumers cannot analyze the size of the
openblas_config_strand may report false positives about buffer overflow. It makes devs triage it for each fork.Suggested fix: it would be great to change
strcpytostrncpyandstrcattostrncat.OpenBLAS/driver/others/openblas_get_config.c
Lines 81 to 93 in 9b3cc78