An Attempt of Static Code Analysis of TinyVM and FreeRDP
cppcheck was utilized for the purposes of this learning exercise; Output in the Appendix of the Page (Static Code Analysis output of TinyVM and FreeRDP)
GitHub Projects:
FreeRDP — https://github.com/FreeRDP/FreeRDP
TinyVM — https://github.com/jakogut/tinyvm
FreeRDP Observations
The code *((trio_long_double_t *)target) = infinity; encased in the following block:
if (flags & FLAGS_LONGDOUBLE) {
*((trio_long_double_t *)target) = infinity;
}
returns a “Possible null pointer deference: target” cppcheck error message because of the negligence to add a block check for null before accessing the variables involved in the operation. I came to this conclusion from evaluation of the blocks surrounding line 6589 and comparing the issue to other reported cppcheck “Possible null pointer deference” errors generated from different code posted on Q&A forums such as Stack Overflow and Quora.
The second cppcheck error was “va_list ‘unused’ used before va_start() was called”, this occurs in 15 different lines because of return TrioFormat calls such as this one:
return TrioFormat(&fd, 0, TrioOutStreamFileDescriptor, format, unused, TrioArrayGetter, args);
This return call is used in different methods with the purpose of printing or scanning for example printing to the file descriptor, printing to the standard output stream, scanning characters from a string, etc.
From evaluating the code, these errors seem like false positives. The missing opens for va_list ‘unused’ aren’t actually missing opens they just mean the code opens in a different function. These are reassignment warnings that don’t take into account the use of the address of the variable so it therefore doesn’t notice the functions attached are in fact using it. So, for this error my recommendations for improvement is to dismiss the errors.
TinyVM Observations
TinyVM only returned two cppcheck issues but to address the most easily fixable error, “Common realloc mistake: ‘breakpoints’ nulled but not freed upon failure” we see the following code on line 49:
case 0x2: breakpoints = realloc(breakpoints, sizeof(struct tdb_breakpoint) * ++num_breakpoints);
These types of memory allocation issues in cppcheck when variables are declared “nulled but not freed upon failure” occur because in the event of the realloc failing then the original memory allocation exists but is leaked because it is no longer referenced by the attached variable (in this case the variable breakpoint). In short terms, they’re memory leaks stemming from reallocation. So, the solution and recommendation for improvement is to keep a handle on the original allocation until the new allocation is verified as valid. Do this with a block of if-else code that checks if new_data is NULL and if it isn’t (thus the else) it’s been verified as valid and you can set the variable equal to new_data in the else statement.
Appendix
TinyVM — Static Code Analysis Output
[libtvm/tvm_preprocessor.c:32]: (error) Memory leak: temp_str
[tdb/tdb.c:49]: (error) Common realloc mistake: ‘breakpoints’ nulled but not freed upon failure
FreeRDP — Static Code Analysis Output
[client/X11/generate_argument_docbook.c:23]: (error) Common realloc mistake: ‘tmp’ nulled but not freed upon failure [libfreerdp/utils/pcap.c:89]: (error) Memory leak: data
[server/Mac/mf_rdpsnd.c:82]: (error) Possible null pointer dereference: agreedFormat [server/shadow/shadow_subsystem.c:218]: (error) Memory leak: xorMaskData [winpr/libwinpr/file/pattern.c:138]: (error) Address of local auto-variable assigned to a function parameter [winpr/libwinpr/file/pattern.c:187]: (error) Address of local auto-variable assigned to a function parameter. [winpr/libwinpr/utils/image.c:314]: (error) Memory leak: data [winpr/libwinpr/utils/sam.c:117]: (error) Memory leak: buffer [winpr/libwinpr/utils/trio/trio.c:6589]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6593]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6597]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6606]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6610]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6614]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6699]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6703]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:6707]: (error) Possible null pointer dereference: target [winpr/libwinpr/utils/trio/trio.c:4239]: (error) va_list ‘unused’ used before va_start() was called [winpr/libwinpr/utils/trio/trio.c:4321]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:4400]: (error) va_list ‘unused’ used before va_start() was called [winpr/libwinpr/utils/trio/trio.c:4470]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:4494]: (error) va_list ‘unused’ used before va_start() was called [winpr/libwinpr/utils/trio/trio.c:4577]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:4672]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:4921]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:5671]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7453]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7544]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7632]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7702]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7726]: (error) va_list ‘unused’ used before va_start() was called. [winpr/libwinpr/utils/trio/trio.c:7812]: (error) va_list ‘unused’ used before va_start() was called.
