OpenVPN / OpenVPN/openvpn-gui

VS code analysis

Open
#647 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
1.8k
Forks
465
PR merge metrics
No merged PRs in 30d

Description

while looking at https://github.com/OpenVPN/openvpn-gui/issues/548
I thought "ok, crash dump is not written, what else can we do".

what if we can use VS code analysis

@selvanair , @lstipakov

image

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the reported analysis findings in localization.c, manage.c, misc.c, openvpn.c, openvpn_config.c, options.c, proxy.c, pkcs11.c, registry.c, service.c, main.c, and the plap files. Run the project's C/C++ code analysis and investigate which warnings represent real issues. The issue does not define which findings must be addressed or what completion looks like.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.