Tags: patch
Further to my prior report it is not the use of variable length array
but the overwriting the end of the array that is the problem. This bug
is repeated multiple times throughout the source file src/detector.c.
It appears that there have been attempts to fix a couple of
occurrences of the bug but even that has been cocked up. There does
not seem to be any understanding by the authors of the code that
strlen() returns the length of a string less the terminating zero.
It is astonishing that this code successfully runs on any architecture
whatsoever given the multiple array overruns!
I attach a patch that fixes many occurrences of the bug. This is
sufficient to get ohcount running on itself on arm64 without crashing.
There are also occurrences of array underruns in the code. For example,
about line 615 of src/detector.c there is the following:
p = line;
while (p < eol) {
if (islower(*p) && p != bof && !isalnum(*(p - 1)) && *(p - 1) != '_') {
On the first iteration of the while loop in the if statement one byte
before the start of array line can be read. That is undefined
behaviour. I have not fixed this in the attached patch, but it should
be reported upstream. Frankly, the code needs a full audit for issues
such as these.
Regards,
Michael.
--- a/src/detector.c
+++ b/src/detector.c
@@ -46,7 +46,7 @@
while (isalnum(*pe)) pe++;
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[79] = '\0';
struct LanguageMap *rl = ohcount_hash_language_from_name(buf, length);
if (rl) return(rl->name);
}
@@ -63,7 +63,7 @@
} while (*p == '-'); // Skip over any switches.
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[79] = '\0';
struct LanguageMap *rl = ohcount_hash_language_from_name(buf, length);
if (rl) return(rl->name);
} else if (strstr(line, "xml")) return(LANG_XML);
@@ -146,7 +146,7 @@
while (pe < eof) {
// Get the contents of the first line.
while (pe < eof && *pe != '\r' && *pe != '\n') pe++;
- length = (pe - p <= sizeof(line)) ? pe - p : sizeof(line);
+ length = (pe - p <= sizeof(line)-1) ? pe - p : sizeof(line)-1;
strncpy(line, p, length);
line[length] = '\0';
if (*line == '#' && *(line + 1) == '!') {
@@ -166,7 +166,7 @@
}
pe = p;
while (!isspace(*pe) && *pe != ';' && pe != strstr(pe, "-*-")) pe++;
- length = (pe - p <= sizeof(buf)) ? pe - p : sizeof(buf);
+ length = (pe - p <= sizeof(buf)-1) ? pe - p : sizeof(buf)-1;
strncpy(buf, p, length);
buf[length] = '\0';
@@ -236,7 +236,7 @@
if (p && pe) {
p += 3;
const int length = pe - p;
- char buf[length];
+ char buf[length+1];
strncpy(buf, p, length);
buf[length] = '\0';
char *eol = buf + strlen(buf);
@@ -550,7 +550,7 @@
// If the directory contains a matching *.m file, likely Objective C.
length = strlen(sourcefile->filename);
if (strcmp(sourcefile->ext, "h") == 0) {
- char path[length];
+ char path[length+1];
strncpy(path, sourcefile->filename, length);
path[length] = '\0';
*(path + length - 1) = 'm';
@@ -572,7 +572,7 @@
while (pe < eof) {
// Get a line at a time.
while (pe < eof && *pe != '\r' && *pe != '\n') pe++;
- length = (pe - p <= sizeof(line)) ? pe - p : sizeof(line);
+ length = (pe - p <= sizeof(line)-1) ? pe - p : sizeof(line)-1;
strncpy(line, p, length);
line[length] = '\0';
char *eol = line + strlen(line);
@@ -594,7 +594,7 @@
while (pe < eol && *pe != '>' && *pe != '"') pe++;
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[80] = '\0';
if (ohcount_hash_is_cppheader(buf, length))
return LANG_CPP;
// Is the extension for the header file a C++ file?
@@ -602,7 +602,7 @@
while (p > line && *(p - 1) != '.') p--;
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[80] = '\0';
struct ExtensionMap *re = ohcount_hash_language_from_ext(buf, length);
if (re && strcmp(re->value, LANG_CPP) == 0)
return LANG_CPP;
@@ -619,7 +619,7 @@
if (!isalnum(*pe) && *pe != '_') {
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[80] = '\0';
if (strcmp(buf, "class") == 0 ||
strcmp(buf, "namespace") == 0 ||
strcmp(buf, "template") == 0 ||
@@ -650,7 +650,7 @@
if (strstr(p, ".") <= pe) {
// Only if the filename has an extension prior to the .in
length = pe - p;
- char buf[length];
+ char buf[length+1];
strncpy(buf, p, length);
buf[length] = '\0';
p = ohcount_sourcefile_get_contents(sourcefile);
@@ -741,7 +741,7 @@
while (pe < eof) {
// Get a line at a time.
while (pe < eof && *pe != '\r' && *pe != '\n') pe++;
- length = (pe - p <= sizeof(line)) ? pe - p : sizeof(line);
+ length = (pe - p <= sizeof(line)-1) ? pe - p : sizeof(line)-1;
strncpy(line, p, length);
line[length] = '\0';
char *eol = line + strlen(line);
@@ -796,7 +796,7 @@
if (!isalnum(*pe)) {
length = pe - p;
strncpy(buf, p, length);
- buf[length] = '\0';
+ buf[80] = '\0';
if (strcmp(buf, "end_try_catch") == 0 ||
strcmp(buf, "end_unwind_protect") == 0 ||
strcmp(buf, "endfunction") == 0 ||
