VS code analysis

[chipitsine]

while looking at #548
I thought "ok, crash dump is not written, what else can we do".

what if we can use VS code analysis

@selvanair , @lstipakov

usually, code analysis is noisy, but some finding can be useful

Code analysis starting...
localization.c
manage.c
misc.c
openvpn.c
openvpn_config.c
options.c
proxy.c
pkcs11.c
registry.c
config_parser.c
service.c
ui_glue.c
stub.c
plap_common.c
plap_provider.c
plap_connection.c
plap_dll.c
Running Code Analysis for C/C++...
C:\i\openvpn-gui\localization.c(246) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string 'formatStr'.: Lines: 203, 204, 205, 208, 209, 215, 216, 222, 225, 232, 238, 239, 243, 245, 246
C:\i\openvpn-gui\localization.c(204) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(227) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(238) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(661) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\manage.c(111) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\manage.c(303) : warning C6385: Reading invalid data from 'line'.: Lines: 210, 211, 212, 214, 215, 220, 256, 258, 263, 264, 269, 270, 277, 279, 280, 281, 282, 283, 286, 287, 289, 290, 291, 292, 293, 296, 303
C:\i\openvpn-gui\misc.c(64) : warning C6001: Using uninitialized memory 'output_len'.: Lines: 54, 55, 57, 64
C:\i\openvpn-gui\misc.c(107) : warning C6001: Using uninitialized memory 'len'.: Lines: 103, 105, 107
C:\i\openvpn-gui\misc.c(113) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(203) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(211) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(211) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(523) : warning C6387: 'sem' could be '0'.: Lines: 519, 523
C:\i\openvpn-gui\misc.c(661) : warning C6031: Return value ignored: 'sscanf'.
C:\i\openvpn-gui\misc.c(969) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\openvpn.c(525) : warning C28159: Consider using 'GetTickCount64' instead of 'GetTickCount'. Reason: GetTickCount overflows roughly every 49 days.  Code that does not take that into account can loop indefinitely.  GetTickCount64 operates on 64 bit values and does not have that problem
C:\i\openvpn-gui\openvpn.c(1869) : warning C6031: Return value ignored: 'ReadFileEx'.
C:\i\openvpn-gui\openvpn.c(2660) : warning C6258: Using TerminateThread does not allow proper thread clean up.
C:\i\openvpn-gui\openvpn.c(2667) : warning C6258: Using TerminateThread does not allow proper thread clean up.
C:\i\openvpn-gui\openvpn.c(2809) : warning C6248: Setting a SECURITY_DESCRIPTOR's DACL to NULL will result in an unprotected object.
C:\i\openvpn-gui\openvpn.c(3067) : warning C6248: Setting a SECURITY_DESCRIPTOR's DACL to NULL will result in an unprotected object.
C:\i\openvpn-gui\openvpn.c(3087) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string 'pwd'.: Lines: 3041, 3042, 3043, 3044, 3045, 3046, 3047, 3048, 3049, 3050, 3052, 3053, 3056, 3057, 3062, 3067, 3074, 3079, 3086, 3087
C:\i\openvpn-gui\openvpn_config.c(203) : warning C28182: Dereferencing NULL pointer. 'o.groups' contains the same NULL value as 'realloc()`194' did. : Lines: 186, 191, 193, 194, 195, 197, 198, 200, 203
C:\i\openvpn-gui\options.c(448) : warning C6011: Dereferencing NULL pointer 'argv'. : Lines: 390, 391, 392, 397, 402, 407, 409, 411, 409, 414, 416, 422, 423, 424, 428, 433, 438, 448
C:\i\openvpn-gui\options.c(794) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\options.c(799) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\proxy.c(128) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\proxy.c(133) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\pkcs11.c(555) : warning C6387: '_Param_(6)' could be '0'.: Lines: 547, 548, 555
C:\i\openvpn-gui\pkcs11.c(653) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\registry.c(459) : warning C6011: Dereferencing NULL pointer 'data'. : Lines: 439, 440, 441, 443, 446, 447, 452, 453, 459
C:\i\openvpn-gui\service.c(71) : warning C6387: 'schService' could be '0'.: Lines: 44, 45, 46, 47, 50, 52, 57, 61, 71
C:\i\openvpn-gui\plap\ui_glue.c(327) : warning C6258: Using TerminateThread does not allow proper thread clean up.
C:\i\openvpn-gui\plap\plap_dll.h(39) : warning C28251: Inconsistent annotation for 'DllCanUnloadNow': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1397). 
C:\i\openvpn-gui\plap\plap_dll.h(41) : warning C28251: Inconsistent annotation for 'DllGetClassObject': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1394). 
C:\i\openvpn-gui\plap\plap_dll.h(39) : warning C28251: Inconsistent annotation for 'DllCanUnloadNow': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1397). 
C:\i\openvpn-gui\plap\plap_dll.h(41) : warning C28251: Inconsistent annotation for 'DllGetClassObject': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1394). 
C:\i\openvpn-gui\plap\plap_dll.h(39) : warning C28251: Inconsistent annotation for 'DllCanUnloadNow': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1397). 
C:\i\openvpn-gui\plap\plap_dll.h(41) : warning C28251: Inconsistent annotation for 'DllGetClassObject': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1394). 
C:\i\openvpn-gui\plap\plap_dll.c(196) : warning C28251: Inconsistent annotation for 'DllCanUnloadNow': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1397). Note: There are several prototypes for this function. This warning compares the first with instance number 3. 
C:\i\openvpn-gui\plap\plap_dll.c(219) : warning C28251: Inconsistent annotation for 'DllGetClassObject': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\combaseapi.h(1394). Note: There are several prototypes for this function. This warning compares the first with instance number 3. 
access.c
echo.c
env_set.c
localization.c
main.c
manage.c
misc.c
openvpn.c
openvpn_config.c
options.c
proxy.c
registry.c
save_pass.c
scripts.c
service.c
tray.c
viewlog.c
as.c
pkcs11.c
config_parser.c
Running Code Analysis for C/C++...
C:\i\openvpn-gui\echo.c(218) : warning C26451: Arithmetic overflow: Using operator '*' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '*' to avoid overflow (io.2).
C:\i\openvpn-gui\echo.c(379) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(246) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string 'formatStr'.: Lines: 203, 204, 205, 208, 209, 215, 216, 222, 225, 232, 238, 239, 243, 245, 246
C:\i\openvpn-gui\localization.c(204) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(227) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(238) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\localization.c(661) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\main.c(164) : warning C28251: Inconsistent annotation for 'wWinMain': this instance has no annotations. See c:\program files (x86)\windows kits\10\include\10.0.19041.0\um\winbase.h(1019). 
C:\i\openvpn-gui\main.c(704) : warning C6011: Dereferencing NULL pointer 'c'. : Lines: 588, 589, 590, 592, 651, 653, 656, 660, 664, 668, 672, 674, 690, 694, 698, 702, 704
C:\i\openvpn-gui\main.c(812) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\manage.c(111) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\manage.c(303) : warning C6385: Reading invalid data from 'line'.: Lines: 210, 211, 212, 214, 215, 220, 256, 258, 263, 264, 269, 270, 277, 279, 280, 281, 282, 283, 286, 287, 289, 290, 291, 292, 293, 296, 303
C:\i\openvpn-gui\misc.c(64) : warning C6001: Using uninitialized memory 'output_len'.: Lines: 54, 55, 57, 64
C:\i\openvpn-gui\misc.c(107) : warning C6001: Using uninitialized memory 'len'.: Lines: 103, 105, 107
C:\i\openvpn-gui\misc.c(113) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(203) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(211) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(211) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\misc.c(523) : warning C6387: 'sem' could be '0'.: Lines: 519, 523
C:\i\openvpn-gui\misc.c(661) : warning C6031: Return value ignored: 'sscanf'.
C:\i\openvpn-gui\misc.c(969) : warning C26451: Arithmetic overflow: Using operator '-' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '-' to avoid overflow (io.2).
C:\i\openvpn-gui\openvpn.c(525) : warning C28159: Consider using 'GetTickCount64' instead of 'GetTickCount'. Reason: GetTickCount overflows roughly every 49 days.  Code that does not take that into account can loop indefinitely.  GetTickCount64 operates on 64 bit values and does not have that problem
C:\i\openvpn-gui\openvpn.c(1869) : warning C6031: Return value ignored: 'ReadFileEx'.
C:\i\openvpn-gui\openvpn.c(2660) : warning C6258: Using TerminateThread does not allow proper thread clean up.
C:\i\openvpn-gui\openvpn.c(2667) : warning C6258: Using TerminateThread does not allow proper thread clean up.
C:\i\openvpn-gui\openvpn.c(2809) : warning C6248: Setting a SECURITY_DESCRIPTOR's DACL to NULL will result in an unprotected object.
C:\i\openvpn-gui\openvpn.c(3067) : warning C6248: Setting a SECURITY_DESCRIPTOR's DACL to NULL will result in an unprotected object.
C:\i\openvpn-gui\openvpn.c(3087) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string 'pwd'.: Lines: 3041, 3042, 3043, 3044, 3045, 3046, 3047, 3048, 3049, 3050, 3052, 3053, 3056, 3057, 3062, 3067, 3074, 3079, 3086, 3087
C:\i\openvpn-gui\openvpn_config.c(203) : warning C28182: Dereferencing NULL pointer. 'o.groups' contains the same NULL value as 'realloc()`194' did. : Lines: 186, 191, 193, 194, 195, 197, 198, 200, 203
C:\i\openvpn-gui\options.c(448) : warning C6011: Dereferencing NULL pointer 'argv'. : Lines: 390, 391, 392, 397, 402, 407, 409, 411, 409, 414, 416, 422, 423, 424, 428, 433, 438, 448
C:\i\openvpn-gui\options.c(794) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\options.c(799) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\proxy.c(128) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\proxy.c(133) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
C:\i\openvpn-gui\registry.c(459) : warning C6011: Dereferencing NULL pointer 'data'. : Lines: 439, 440, 441, 443, 446, 447, 452, 453, 459
C:\i\openvpn-gui\service.c(71) : warning C6387: 'schService' could be '0'.: Lines: 44, 45, 46, 47, 50, 52, 57, 61, 71
C:\i\openvpn-gui\tray.c(150) : warning C26451: Arithmetic overflow: Using operator '+' on a 4 byte value and then casting the result to a 8 byte value. Cast the value to the wider type before calling operator '+' to avoid overflow (io.2).
C:\i\openvpn-gui\tray.c(462) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string '(first_conn)?(msg_connected):((unsigned short [3])L", ")'.: Lines: 445, 446, 447, 448, 449, 450, 452, 453, 454, 456, 457, 459, 462
C:\i\openvpn-gui\tray.c(475) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string '(first_conn)?(msg_connecting):((unsigned short [3])L", ")'.: Lines: 445, 446, 447, 448, 449, 450, 452, 453, 454, 456, 457, 459, 457, 469, 470, 472, 475
C:\i\openvpn-gui\tray.c(513) : warning C6053: The prior call to 'wcsncpy' might not zero-terminate string 'msg'.: Lines: 445, 446, 447, 448, 449, 450, 452, 453, 454, 456, 457, 469, 470, 481, 497, 498, 502, 507, 508, 509, 510, 511, 512, 513
C:\i\openvpn-gui\viewlog.c(57) : warning C6031: Return value ignored: 'CoInitializeEx'.
C:\i\openvpn-gui\viewlog.c(119) : warning C6031: Return value ignored: 'CoInitializeEx'.
C:\i\openvpn-gui\as.c(601) : warning C6001: Using uninitialized memory 'tmp'.: Lines: 433, 434, 435, 436, 437, 440, 441, 444, 449, 450, 457, 458, 459, 462, 464, 465, 471, 472, 473, 474, 480, 481, 490, 491, 495, 542, 543, 544, 546, 549, 551, 556, 557, 558, 590, 597, 599, 600, 601
C:\i\openvpn-gui\as.c(602) : warning C6054: String 'tmp' might not be zero-terminated.: Lines: 433, 434, 435, 436, 437, 440, 441, 444, 449, 450, 457, 458, 459, 462, 464, 465, 471, 472, 473, 474, 480, 481, 490, 491, 495, 542, 543, 544, 546, 549, 551, 556, 557, 558, 590, 597, 599, 600, 601, 602
C:\i\openvpn-gui\pkcs11.c(555) : warning C6387: '_Param_(6)' could be '0'.: Lines: 547, 548, 555
C:\i\openvpn-gui\pkcs11.c(653) : warning C26454: Arithmetic overflow: '-' operation produces a negative unsigned result at compile time (io.5).
test_plap.cpp
C:\i\openvpn-gui\plap\test_plap.cpp(52) : warning C26495: Variable 'MyQueryContinue::timeout' is uninitialized. Always initialize a member variable (type.6).
C:\i\openvpn-gui\plap\test_plap.cpp(129) : warning C6031: Return value ignored: '_setmode'.
C:\i\openvpn-gui\plap\test_plap.cpp(130) : warning C6031: Return value ignored: '_setmode'.
plap_common.c

Code analysis complete.

[selvanair]

Thanks for this, it does point out a few things that we could improve.

But mostly a lot of bogus warnings and some potential overflows that are unlikely to happen in normal use. Nothing that could lead to a crash as far as I can see.