From 2f7d71a5676aadd16e587b0f32f21b70b9f76f8d Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Fri, 24 Aug 2018 15:23:45 +0500 Subject: [PATCH 1/3] src/Cedar/Cedar.c: resolve "Identical code for different branches", remove unused functions found by coverity, cppcheck [src/Cedar/Cedar.c:1605]: (style) The function 'EnableDebugLog' is never used. [src/Cedar/Cedar.c:858]: (style) The function 'GetUnestablishedConnections' is never used. [src/Cedar/Cedar.c:652]: (style) The function 'InitHiddenPassword' is never used. [src/Cedar/Cedar.c:633]: (style) The function 'IsHiddenPasswordChanged' is never used. [src/Cedar/Cedar.c:393]: (style) The function 'IsInNoSsl' is never used. [src/Cedar/Cedar.c:1785]: (style) The function 'IsLaterBuild' is never used. --- src/Cedar/Cedar.c | 166 ++-------------------------------------------- src/Cedar/Cedar.h | 6 -- 2 files changed, 4 insertions(+), 168 deletions(-) diff --git a/src/Cedar/Cedar.c b/src/Cedar/Cedar.c index 0e5ce36d..aaee7d86 100644 --- a/src/Cedar/Cedar.c +++ b/src/Cedar/Cedar.c @@ -265,23 +265,11 @@ bool IsSupportedWinVer(RPC_WINVER *v) if ((v->VerMajor == 6 && v->VerMinor == 4) || (v->VerMajor == 10 && v->VerMinor == 0)) { - if (v->IsServer == false) + // Windows 10 or Windows Server 2016 + if (v->ServicePack <= 0) { - // Windows 10 (not Windows Server 2016) - if (v->ServicePack <= 0) - { - // SP0 only - return true; - } - } - else - { - // Windows Server 2016 - if (v->ServicePack <= 0) - { - // SP0 only - return true; - } + // SP0 only + return true; } } @@ -389,34 +377,6 @@ int CompareNoSslList(void *p1, void *p2) return CmpIpAddr(&n1->IpAddress, &n2->IpAddress); } -// Check whether the specified IP address is in Non-SSL connection list -bool IsInNoSsl(CEDAR *c, IP *ip) -{ - bool ret = false; - // Validate arguments - if (c == NULL || ip == NULL) - { - return false; - } - - LockList(c->NonSslList); - { - NON_SSL *n = SearchNoSslList(c, ip); - - if (n != NULL) - { - if (n->EntryExpires > Tick64() && n->Count > NON_SSL_MIN_COUNT) - { - n->EntryExpires = Tick64() + (UINT64)NON_SSL_ENTRY_EXPIRES; - ret = true; - } - } - } - UnlockList(c->NonSslList); - - return ret; -} - // Decrement connection count of Non-SSL connection list entry void DecrementNoSsl(CEDAR *c, IP *ip, UINT num_dec) { @@ -629,37 +589,6 @@ UINT64 GetTrafficPacketNum(TRAFFIC *t) t->Send.BroadcastCount + t->Send.UnicastCount; } -// Get whether hidden password is changed in UI -bool IsHiddenPasswordChanged(char *str) -{ - // Validate arguments - if (str == NULL) - { - return true; - } - - if (StrCmpi(str, HIDDEN_PASSWORD) == 0) - { - return true; - } - else - { - return false; - } -} - -// Initialize hidden password in UI -void InitHiddenPassword(char *str, UINT size) -{ - // Validate arguments - if (str == NULL) - { - return; - } - - StrCpy(str, size, HIDDEN_PASSWORD); -} - // Check whether the certificate is signed by CA which is trusted by the hub bool CheckSignatureByCaLinkMode(SESSION *s, X *x) { @@ -854,47 +783,6 @@ void DelConnection(CEDAR *cedar, CONNECTION *c) UnlockList(cedar->ConnectionList); } -// Get the number of unestablished connections -UINT GetUnestablishedConnections(CEDAR *cedar) -{ - UINT i, ret; - // Validate arguments - if (cedar == NULL) - { - return 0; - } - - ret = 0; - - LockList(cedar->ConnectionList); - { - for (i = 0;i < LIST_NUM(cedar->ConnectionList);i++) - { - CONNECTION *c = LIST_DATA(cedar->ConnectionList, i); - - switch (c->Type) - { - case CONNECTION_TYPE_CLIENT: - case CONNECTION_TYPE_INIT: - case CONNECTION_TYPE_LOGIN: - case CONNECTION_TYPE_ADDITIONAL: - switch (c->Status) - { - case CONNECTION_STATUS_ACCEPTED: - case CONNECTION_STATUS_NEGOTIATION: - case CONNECTION_STATUS_USERAUTH: - ret++; - break; - } - break; - } - } - } - UnlockList(cedar->ConnectionList); - - return ret + Count(cedar->AcceptingSockets); -} - // Add connection to Cedar void AddConnection(CEDAR *cedar, CONNECTION *c) { @@ -1601,18 +1489,6 @@ void SetCedarCert(CEDAR *c, X *server_x, K *server_k) Unlock(c->lock); } -// Enable debug log -void EnableDebugLog(CEDAR *c) -{ - // Validate arguments - if (c == NULL || c->DebugLog != NULL) - { - return; - } - - c->DebugLog = NewLog("cedar_debug_log", "cedar", LOG_SWITCH_NO); -} - // Set the Cedar into VPN Bridge mode void SetCedarVpnBridge(CEDAR *c) { @@ -1781,40 +1657,6 @@ CEDAR *NewCedar(X *server_x, K *server_k) return c; } -// Check whether the Cedar was build after the specified date -bool IsLaterBuild(CEDAR *c, UINT64 t) -{ - SYSTEMTIME sb, st; - UINT64 b; - // Validate arguments - if (c == NULL) - { - return false; - } - - Zero(&sb, sizeof(sb)); - Zero(&st, sizeof(st)); - - UINT64ToSystem(&sb, c->BuiltDate); - UINT64ToSystem(&st, t); - - // Ignore time of the day - sb.wHour = sb.wMinute = sb.wSecond = sb.wMilliseconds = 0; - st.wHour = st.wMinute = st.wSecond = st.wMilliseconds = 0; - - b = SystemToUINT64(&sb); - t = SystemToUINT64(&st); - - if (b > t) - { - return true; - } - else - { - return false; - } -} - // Cumulate traffic size void AddTraffic(TRAFFIC *dst, TRAFFIC *diff) { diff --git a/src/Cedar/Cedar.h b/src/Cedar/Cedar.h index 7db8ce31..74d53eda 100644 --- a/src/Cedar/Cedar.h +++ b/src/Cedar/Cedar.h @@ -1211,7 +1211,6 @@ void DelHubEx(CEDAR *c, HUB *h, bool no_lock); void StopAllHub(CEDAR *c); void StopAllConnection(CEDAR *c); void AddConnection(CEDAR *cedar, CONNECTION *c); -UINT GetUnestablishedConnections(CEDAR *cedar); void DelConnection(CEDAR *cedar, CONNECTION *c); void SetCedarCipherList(CEDAR *cedar, char *name); void InitCedar(); @@ -1225,11 +1224,8 @@ void InitNetSvcList(CEDAR *cedar); void FreeNetSvcList(CEDAR *cedar); int CompareNetSvc(void *p1, void *p2); char *GetSvcName(CEDAR *cedar, bool udp, UINT port); -void InitHiddenPassword(char *str, UINT size); -bool IsHiddenPasswordChanged(char *str); UINT64 GetTrafficPacketSize(TRAFFIC *t); UINT64 GetTrafficPacketNum(TRAFFIC *t); -void EnableDebugLog(CEDAR *c); void StartCedarLog(); void StopCedarLog(); int CompareNoSslList(void *p1, void *p2); @@ -1239,13 +1235,11 @@ bool AddNoSsl(CEDAR *c, IP *ip); void DecrementNoSsl(CEDAR *c, IP *ip, UINT num_dec); void DeleteOldNoSsl(CEDAR *c); NON_SSL *SearchNoSslList(CEDAR *c, IP *ip); -bool IsInNoSsl(CEDAR *c, IP *ip); void FreeTinyLog(TINY_LOG *t); void WriteTinyLog(TINY_LOG *t, char *str); TINY_LOG *NewTinyLog(); void GetWinVer(RPC_WINVER *v); bool IsSupportedWinVer(RPC_WINVER *v); -bool IsLaterBuild(CEDAR *c, UINT64 t); SOCK *GetInProcListeningSock(CEDAR *c); SOCK *GetReverseListeningSock(CEDAR *c); void GetCedarVersion(char *tmp, UINT size); From a58d26f12529f40598bfb66f1578b49913d4c180 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Fri, 24 Aug 2018 15:25:34 +0500 Subject: [PATCH 2/3] src/Cedar/IPsec_IKE.c: resolve null pointer dereference found by coverity, remove unused variable [src/Cedar/IPsec_IKE.c:4332] -> [src/Cedar/IPsec_IKE.c:4332]: (style) Same expression on both sides of '||'. [src/Cedar/IPsec_IKE.c:1665]: (style) Variable 'zero' is assigned a value that is never used. --- src/Cedar/IPsec_IKE.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Cedar/IPsec_IKE.c b/src/Cedar/IPsec_IKE.c index dfe56168..a35d8467 100644 --- a/src/Cedar/IPsec_IKE.c +++ b/src/Cedar/IPsec_IKE.c @@ -1662,7 +1662,6 @@ void StartQuickMode(IKE_SERVER *ike, IKE_CLIENT *c) UINT spi; UINT spi_be; UCHAR hash1[IKE_MAX_HASH_SIZE]; - UCHAR zero = 0; DH_CTX *dh = NULL; UCHAR dummy_hash_data[IKE_MAX_HASH_SIZE]; @@ -3842,6 +3841,10 @@ bool IkeIsVendorIdExists(IKE_PACKET *p, char *str) for (i = 0;i < num;i++) { IKE_PACKET_PAYLOAD *payload = IkeGetPayload(p->PayloadList, IKE_PAYLOAD_VENDOR_ID, i); + if (payload == NULL) + { + return false; + } if (CompareBuf(payload->Payload.VendorId.Data, buf)) { @@ -4325,7 +4328,7 @@ IKE_CLIENT *SearchOrCreateNewIkeClientForIkePacket(IKE_SERVER *ike, IP *client_i { IKE_CLIENT *c; // Validate arguments - if (ike == NULL || pr == NULL || client_ip == NULL || server_ip == NULL || client_port == 0 || server_port == 0 || pr == NULL) + if (ike == NULL || pr == NULL || client_ip == NULL || server_ip == NULL || client_port == 0 || server_port == 0) { return NULL; } From dcd03476c4bc4efaf28ffc3144831f7fc06e0789 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Fri, 24 Aug 2018 15:26:14 +0500 Subject: [PATCH 3/3] src/Cedar/Connection: resolve null pointer dereference found by coverity, remove unused function [src/Cedar/Connection.c:2861]: (style) The function 'InitTcpSockRc4Key' is never used. --- src/Cedar/Connection.c | 30 +----------------------------- src/Cedar/Connection.h | 1 - 2 files changed, 1 insertion(+), 30 deletions(-) diff --git a/src/Cedar/Connection.c b/src/Cedar/Connection.c index 772463a8..fd063244 100644 --- a/src/Cedar/Connection.c +++ b/src/Cedar/Connection.c @@ -1171,7 +1171,7 @@ void ConnectionSend(CONNECTION *c, UINT64 now) UINT j; QUEUE *q; - if (s->UdpAccel != NULL) + if (s != NULL && s->UdpAccel != NULL) { UdpAccelSetTick(s->UdpAccel, now); } @@ -2857,34 +2857,6 @@ TCPSOCK *NewTcpSock(SOCK *s) return ts; } -// Set a encryption key for the TCP socket -void InitTcpSockRc4Key(TCPSOCK *ts, bool server_mode) -{ - RC4_KEY_PAIR *pair; - CRYPT *c1, *c2; - // Validate arguments - if (ts == NULL) - { - return; - } - - pair = &ts->Rc4KeyPair; - - c1 = NewCrypt(pair->ClientToServerKey, sizeof(pair->ClientToServerKey)); - c2 = NewCrypt(pair->ServerToClientKey, sizeof(pair->ServerToClientKey)); - - if (server_mode) - { - ts->RecvKey = c1; - ts->SendKey = c2; - } - else - { - ts->SendKey = c1; - ts->RecvKey = c2; - } -} - // Release of TCP socket void FreeTcpSock(TCPSOCK *ts) { diff --git a/src/Cedar/Connection.h b/src/Cedar/Connection.h index 1c95e088..2b2c570c 100644 --- a/src/Cedar/Connection.h +++ b/src/Cedar/Connection.h @@ -351,7 +351,6 @@ void DisconnectUDPSockets(CONNECTION *c); void PutUDPPacketData(CONNECTION *c, void *data, UINT size); void SendDataWithUDP(SOCK *s, CONNECTION *c); void InsertReceivedBlockToQueue(CONNECTION *c, BLOCK *block, bool no_lock); -void InitTcpSockRc4Key(TCPSOCK *ts, bool server_mode); UINT TcpSockRecv(SESSION *s, TCPSOCK *ts, void *data, UINT size); UINT TcpSockSend(SESSION *s, TCPSOCK *ts, void *data, UINT size); void WriteSendFifo(SESSION *s, TCPSOCK *ts, void *data, UINT size);