*** CID 358434: Null pointer dereferences (REVERSE_INULL)
/src/Cedar/Proto.c: 451 in ProtoHandleDatagrams()
445 void ProtoHandleDatagrams(UDPLISTENER *listener, LIST *datagrams)
446 {
447 UINT i;
448 HASH_LIST *sessions;
449 PROTO *proto = listener->Param;
450
>>> CID 358434: Null pointer dereferences (REVERSE_INULL)
>>> Null-checking "listener" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
451 if (proto == NULL || listener == NULL || datagrams == NULL)
452 {
453 return;
454 }
455
456 sessions = proto->Sessions;
As a side effect, the DH parameter is now applied to the TCP server as well.
Previously, the default value was always used, ignoring the one from the configuration.
When a datagram is received, the matching session is looked up in a hash list; if it's not found, a new session is created.
This method allows to use a single UDP port for multiple protocols, as we do with TCP.
Also, each session has its own dedicated thread, used to process the received datagrams and generate the ones that are then sent through the UDP listener.
In addition to guaranteeing constant performance, separate threads also prevent a single one from blocking all sessions.
This allows to stop a UDP listener without deleting it.
It's especially useful when no datagrams should be received anymore, but there are other threads accessing the listener.
- An additional parameter is added to IsPacketForMe(), used to specify the protocol type (currently either TCP or UDP).
- SupportedModes() is dropped because it's now redundant.
- IsOk() and EstablishedSessions() are dropped because error checking should be handled by the implementation.
- ProtoImplDetect() now takes a buffer and its size rather than a SOCK, so that it can be used to detect UDP protocols.
- The OpenVPN toggle check is moved to ProtoImplDetect(), so that we don't have to duplicate it once UDP support is implemented.
The PROTO structure is now used to identify the system as a whole, rather than a single protocol. It's stored and initialized in Server.
ProtoCompare(), ProtoAdd() and ProtoDetected() are renamed to make the difference between PROTO and PROTO_IMPL more clear.
ProtoGet() and ProtoNum() are removed because the related list can now be accessed directly by Server.
From https://community.openvpn.net/openvpn/wiki/Openvpn23ManPage:
--block-outside-dns
Block DNS servers on other network adapters to prevent DNS leaks.
This option prevents any application from accessing TCP or UDP port 53 except one inside the tunnel.
It uses Windows Filtering Platform (WFP) and works on Windows Vista or later.
This option is considered unknown on non-Windows platforms and unsupported on Windows XP, resulting in fatal error.
You may want to use --setenv opt or --ignore-unknown-option (not suitable for Windows XP) to ignore said error.
Note that pushing unknown options from server does not trigger fatal errors.
On uClibc, the ifaddrs.h support is optional. While the default
Buildroot uClibc configuration has it enabled, some external
toolchains may not. Therefore this patch detects that and adjusts
softether usage of ifaddrs accordingly.
Based on an initial patch from Bernd Kuhls.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[Retrieved from:
https://git.buildroot.net/buildroot/tree/package/softether/0009-uclibc-ai-addrconfig.patch]
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Formerly, SKIP_CPU_FEATURES is automatically detected by system
processor. However, "^(armv7l|aarch64|s390x)$" does not cover all
processors that cpu_features should be skipped.
"armv6", "armv7", "mips", "mips64" on FreeBSD are examples [1]
that cpu_features is not correctly skipped.
This change intends to build SoftEther without any modifications on
CMakeLists.txt on such processors.
cmake . -DSKIP_CPU_FEATURES=1
[1] https://www.freebsd.org/platforms/
DbDir : directory to store files such as vpn_server.config and backups etc
LogDir : directory to write logs (sub directories is created in this dir)
PidDir : directory to put PID files such as .ctl-* .pid-* .VPN-*
Currently the systemd service unit files are installed
into /lib/systemd/system if that directory exists. This
might not be optimal for every user, e.g. when the build
system is not the target system or when building as an
unprivileged user using CMAKE_INSTALL_PREFIX.
Make this configurable by adding a cached cmake variable
CMAKE_INSTALL_SYSTEMD_UNITDIR. Usage:
- install unit files into /lib/systemd/system if it exists (old
behavior)
cmake
- don't install unit files
cmake -D CMAKE_INSTALL_SYSTEMD_UNITDIR=
- install into absolute path
cmake -D CMAKE_INSTALL_SYSTEMD_UNITDIR=/path
- install into path relative to ${CMAKE_INSTALL_PREFIX}
cmake -D CMAKE_INSTALL_SYSTEMD_UNITDIR=path
The function has been greatly improved, here are some of the changes:
- The required SESSION (c->Session) parameter is checked correctly: the function returns immediately in case it's NULL. Previously, the function didn't return in case the parameter was NULL; multiple checks were in place, but not in all instances where the parameter was dereferenced.
- The resolved IP address is cached with all proxy types.
- The "RestoreServerNameAndPort" variable is documented.
- The Debug() messages have been improved.
This commit moves the generic (not related to our protocol) proxy stuff from Cedar to Mayaqua, in dedicated files.
The functions are refactored so that they all have the same arguments and follow the same logic.
Dedicated error codes are added, in order to indicate clearly why the function(s) failed.
Coverity Scan detected an out-of-bounds access issue: OvsProcessData() checked whether the payload size was bigger than the size of the buffer, instead of checking whether the entire packet size (payload size + 2 bytes) was, resulting in an out-of-bounds access in case the payload size is bigger than 1998.
This commit also improves the variable names, the comments and adds two Debug() lines.
OvsDecrypt() returns 0 when it fails, resulting in "size" rolling over with an end result of 4294967292.
This commit fixes the issue by checking whether "size" is greater than sizeof(UINT) before performing the subtraction.
The bug was caused by a typo in the StrCpy() call: the source buffer was the same as the destination one, meaning that the function didn't do anything.
- Fixed the RADIUS PEAP client to use the standard TLS versioning.
- Implementation of a function to fix the MAC address of L3 VPN protocol by entering e.g. "MAC: 112233445566" in the "Notes" field of the user information.
- Implementation of a function to fix the virtual MAC address to be assigned to the L3 VPN client as a string attribute from RADIUS server when authentication.
Hardcoded paths are used in log file enumeration such as LogFileList
command or GenerateEraseFileList function to delete old log files when
disk free space is lacking.
Fixes: SoftEtherVPN/SoftEtherVPN#972
If SecureNAT is enabled and the hostname of the server
is longer than 16characters, every NETBIOS name resolution
query triggers the buffer overflow. If the server was built
with stack protection, the process will be killed.
This commit adds a protocol interface to the server, its purpose is to manage TCP connections and the various third-party protocols.
More specifically, ProtoHandleConnection() takes care of exchanging the packets between the local and remote endpoint; the protocol implementation only has to parse them and act accordingly.
The interface knows which protocol is the connection for by calling IsPacketForMe(), a function implemented for each protocol.
My previous patch used a wrong if directive, which disabled removed
(de)initialization and threading for LibreSSL. This most likely causes
issues at runtime.
A race condition in the DHCP server caused it to offer the same IP address to multiple clients when they connected at the same time, because an offered IP address was considered free until the final step (DHCP_ACK).
This commit introduces a list to keep track of the pending leases created during DHCP_OFFER, so that an IP address is guaranteed to be offered to a single client.
StrCat() appends a string to an already existing string. In order to know where the existing string ends, it uses StrLen() which in turn uses strlen(), a function considered unsafe because it doesn't stop until it finds the null character.
Since the string was allocated but not initialized, StrCat() was either:
- Working correctly.
- Copying only a part of the string.
- Making the program crash via strlen().
The fix consists in using StrCpy(), which starts writing at the beginning of the string.
SSL_free() also frees the associated context.
d6c3c1896c/ssl/ssl_lib.c (L1209)
From https://www.openssl.org/docs/man1.1.1/man3/SSL_free.html:
"SSL_free() also calls the free()ing procedures for indirectly affected items, if applicable: the buffering BIO, the read and write BIOs, cipher lists specially created for this ssl, the SSL_SESSION. Do not explicitly free these indirectly freed up items before or after calling SSL_free(), as trying to free things twice may lead to program failure."
found by cppcheck
[src/Cedar/DDNS.c:656]: (style) Condition 'ret==NULL' is always true
[src/Cedar/DDNS.c:515] -> [src/Cedar/DDNS.c:640]: (style) The expression 'use_https == false' is always true because 'use_https' and 'false' represent the same value.
[src/Cedar/DDNS.c:516] -> [src/Cedar/DDNS.c:648]: (style) The expression 'no_cert_verify == false' is always true because 'no_cert_verify' and 'false' represent the same value.
[src/Cedar/DDNS.c:816] -> [src/Cedar/DDNS.c:860]: (style) The expression 'no_cert_verify == false' is always true because 'no_cert_verify' and 'false' represent the same value.
[src/Cedar/DDNS.c:530]: (style) Variable 'use_vgs' is assigned a value that is never used.
[src/Cedar/DDNS.c:497]: (style) The function 'DCUpdateNow' is never used.
found by cppcheck
[src/Cedar/Account.c:523]: (style) The function 'GetUserPolicy' is never used.
[src/Cedar/Account.c:163]: (style) The function 'NormalizePolicyName' is never used.
found by cppcheck
[src/Cedar/Command.c:23220] -> [src/Cedar/Command.c:23232]: (style) Variable 'len' is reassigned a value before the old one has been used.
found by cppcheck
[src/Cedar/CM.c:4509]: (style) Variable 'easy' is assigned a value that is never used.
[src/Cedar/CM.c:4547]: (style) Variable 'hub_name' is assigned a value that is never used.
[src/Cedar/CM.c:4609]: (style) Variable 'is_account' is assigned a value that is never used.
[src/Cedar/CM.c:8545]: (style) The function 'CmLoadK' is never used.
To fix the bug of OpenVPN 2.4.6 and particular version of kernel mode TAP driver on Linux, the TAP device must be up after the OpenVPN client is connected. However there is no direct push instruction to do so to OpenVPN client. Therefore we push the dummy IPv4 address (RFC7600) to the OpenVPN client to enforce the TAP driver UP state.
