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.

Data Scientist

Get the Medium app

A button that says 'Download on the App Store', and if clicked it will lead you to the iOS App store
A button that says 'Get it on, Google Play', and if clicked it will lead you to the Google Play store