AMajor/SoftEtherVPN
1
0
Fork 0
mirror of https://github.com/SoftEtherVPN/SoftEtherVPN.git synced 2024-09-18 01:33:00 +03:00
Code Releases Activity

7 missing memory boundaries checks and similar memory problems. There are no risk of arbitrary code execution or intrusion on these bugs in my analysis. However, these problems may lead to crash the running server process. So these bugs must be fixed.

Browse Source 
Buffer overread in ParseL2TPPacket()
Memory corruption in IcmpParseResult
Missing bounds check in ParseUDP() can lead to invalid memory access
Out-of-bounds read in IPsec_PPP.c (unterminated string buffer)
Overlapping parameters to memcpy() via StrToIp6()
PACK ReadValue() crash vulnerability
Potential use of uninitialized memory via IPToInAddr6()

4 memory leaks. While the amount of leakage is very small per time, these bugs can finally cause process crash by out of memory. So these bugs must be fixed.

Memory leak in NnReadDnsRecord
Memory leak in RadiusLogin()
Memory leak via ParsePacketIPv4WithDummyMacHeader
Remote memory leak in OpenVPN server code

1 coding improvement. This is not a bug, however, I fixed the code to avoid furture misunderstanding.

RecvAll can return success on failure (leading to use of uninitialized memory)

Contributors for this bugfix:

- Max Planck Institute for Molecular Genetics
- Guido Vranken
This commit is contained in:
Daiyuu Nobori 2018-01-15 10:25:10 +09:00
parent 8edbcd4c0d
commit 7de986dcca
12 changed files with 106 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Cedar/IPsec_L2TP.c
View File

@ -792,6 +792,12 @@ L2TP_PACKET *ParseL2TPPacket(UDPPACKET *p)
size -= 2;
a.DataSize = a.Length - 6;
if (a.DataSize > size)
{
goto LABEL_ERROR;
}
a.Data = Clone(buf, a.DataSize);
buf += a.DataSize;

4
src/Cedar/IPsec_PPP.c
View File

@ -291,7 +291,7 @@ void PPPThread(THREAD *thread, void *param)
ReadBuf(b, client_response_buffer, 49);
Zero(username_tmp, sizeof(username_tmp));
ReadBuf(b, username_tmp, sizeof(username_tmp));
ReadBuf(b, username_tmp, sizeof(username_tmp) - 1);
Debug("First MS-CHAPv2: id=%s\n", username_tmp);
@ -977,7 +977,7 @@ PPP_PACKET *PPPProcessRequestPacket(PPP_SESSION *p, PPP_PACKET *req)
ReadBuf(b, client_response_buffer, 49);
Zero(username_tmp, sizeof(username_tmp));
ReadBuf(b, username_tmp, sizeof(username_tmp));
ReadBuf(b, username_tmp, sizeof(username_tmp) - 1);
client_challenge_16 = client_response_buffer + 0;
client_response_24 = client_response_buffer + 16 + 8;

2
src/Cedar/Interop_OpenVPN.c
View File

@ -2845,7 +2845,7 @@ bool OvsPerformTcpServer(CEDAR *cedar, SOCK *sock)
{
void *ptr = FifoPtr(tcp_recv_fifo);
USHORT packet_size = READ_USHORT(ptr);
if (packet_size <= OPENVPN_TCP_MAX_PACKET_SIZE)
if (packet_size != 0 && packet_size <= OPENVPN_TCP_MAX_PACKET_SIZE)
{
UINT total_len = (UINT)packet_size + sizeof(USHORT);
if (r >= total_len)

7
src/Cedar/Radius.c
View File

@ -1827,6 +1827,13 @@ bool RadiusLogin(CONNECTION *c, char *server, UINT port, UCHAR *secret, UINT sec
if (encrypted_password == NULL)
{
// Encryption failure
// Release the ip_list
for(i = 0; i < LIST_NUM(ip_list); i++)
{
IP *tmp_ip = LIST_DATA(ip_list, i);
Free(tmp_ip);
}
ReleaseList(ip_list);
return false;
}

1
src/Cedar/Virtual.c
View File

@ -2250,6 +2250,7 @@ BUF *NnReadDnsRecord(BUF *buf, bool answer, USHORT *ret_type, USHORT *ret_class)
data = Malloc(data_len);
if (ReadBuf(buf, data, data_len) != data_len)
{
Free(data);
return false;
}

15
src/Mayaqua/Memory.c
View File

@ -4313,6 +4313,21 @@ void Copy(void *dst, void *src, UINT size)
memcpy(dst, src, size);
}
// Memory move
void Move(void *dst, void *src, UINT size)
{
// Validate arguments
if (dst == NULL || src == NULL || size == 0 || dst == src)
{
return;
}
// KS
KS_INC(KS_COPY_COUNT);
memmove(dst, src, size);
}
// Memory comparison
int Cmp(void *p1, void *p2, UINT size)
{

1
src/Mayaqua/Memory.h
View File

@ -284,6 +284,7 @@ void *InternalReAlloc(void *addr, UINT size);
void InternalFree(void *addr);
void Copy(void *dst, void *src, UINT size);
void Move(void *dst, void *src, UINT size);
int Cmp(void *p1, void *p2, UINT size);
int CmpCaseIgnore(void *p1, void *p2, UINT size);
void ZeroMem(void *addr, UINT size);

12
src/Mayaqua/Network.c
View File

@ -7373,7 +7373,7 @@ bool StrToIP6(IP *ip, char *str)
if (StartWith(tmp, "[") && EndWith(tmp, "]"))
{
// If the string is enclosed in square brackets, remove brackets
StrCpy(tmp, sizeof(tmp), &tmp[1]);
StrCpyAllowOverlap(tmp, sizeof(tmp), &tmp[1]);
if (StrLen(tmp) >= 1)
{
@ -12691,6 +12691,14 @@ bool RecvAll(SOCK *sock, void *data, UINT size, bool secure)
{
return false;
}
if (ret == SOCK_LATER)
{
// I suppose that this is safe because the RecvAll() function is used only
// if the sock->AsyncMode == true. And the Recv() function may return
// SOCK_LATER only if the sock->AsyncMode == false. Therefore the call of
// Recv() function in the RecvAll() function never returns SOCK_LATER.
return false;
}
recv_size += ret;
if (recv_size >= size)
{
@ -17597,7 +17605,7 @@ void IPToInAddr6(struct in6_addr *addr, IP *ip)
return;
}
Zero(addr, sizeof(struct in_addr));
Zero(addr, sizeof(struct in6_addr));
if (IsIP6(ip))
{

2
src/Mayaqua/Pack.c
View File

@ -354,7 +354,7 @@ VALUE *ReadValue(BUF *b, UINT type)
break;
case VALUE_STR: // ANSI string
len = ReadBufInt(b);
if ((len + 1) > MAX_VALUE_SIZE)
if (len > (MAX_VALUE_SIZE - 1))
{
// Size over
break;

48
src/Mayaqua/Str.c
View File

@ -3346,6 +3346,54 @@ UINT StrCpy(char *dst, UINT size, char *src)
return len;
}
UINT StrCpyAllowOverlap(char *dst, UINT size, char *src)
{
UINT len;
// Validate arguments
if (dst == src)
{
return StrLen(src);
}
if (dst == NULL || src == NULL)
{
if (src == NULL && dst != NULL)
{
if (size >= 1)
{
dst[0] = '\0';
}
}
return 0;
}
if (size == 1)
{
dst[0] = '\0';
return 0;
}
if (size == 0)
{
// Ignore the length
size = 0x7fffffff;
}
// Check the length
len = StrLen(src);
if (len <= (size - 1))
{
Move(dst, src, len + 1);
}
else
{
len = size - 1;
Move(dst, src, len);
dst[len] = '\0';
}
// KS
KS_INC(KS_STRCPY_COUNT);
return len;
}
// Check whether the string buffer is within the specified size
bool StrCheckSize(char *str, UINT size)

1
src/Mayaqua/Str.h
View File

@ -135,6 +135,7 @@ UINT StrSize(char *str);
bool StrCheckLen(char *str, UINT len);
bool StrCheckSize(char *str, UINT size);
UINT StrCpy(char *dst, UINT size, char *src);
UINT StrCpyAllowOverlap(char *dst, UINT size, char *src);
UINT StrCat(char *dst, UINT size, char *src);
UINT StrCatLeft(char *dst, UINT size, char *src);
char ToLower(char c);

18
src/Mayaqua/TcpIp.c
View File

@ -174,14 +174,14 @@ ICMP_RESULT *IcmpParseResult(IP *dest_ip, USHORT src_id, USHORT src_seqno, UCHAR
if (true)
{
UINT ip_header_size = GetIpHeaderSize(recv_buffer, i);
if (ip_header_size >= sizeof(IPV4_HEADER))
if (ip_header_size >= sizeof(IPV4_HEADER) && (ip_header_size <= i))
{
IPV4_HEADER *ipv4 = (IPV4_HEADER *)recv_buffer;
if ((IPV4_GET_VERSION(ipv4) == 4) && (ipv4->Protocol == IP_PROTO_ICMPV4))
{
UINT ip_total_len = (UINT)Endian16(ipv4->TotalLength);
if ((ip_total_len >= sizeof(IPV4_HEADER)) && (ip_total_len <= i))
if ((ip_total_len >= sizeof(IPV4_HEADER)) && (ip_total_len <= i) && (ip_total_len >= ip_header_size))
{
UINT icmp_packet_size = ip_total_len - ip_header_size;
ICMP_HEADER *icmp = (ICMP_HEADER *)(recv_buffer + ip_header_size);
@ -1957,7 +1957,7 @@ void CorrectChecksum(PKT *p)
{
udp->Checksum = 0;
if ((IPV4_GET_FLAGS(v4) & 0x01) == 0)
if ((IPV4_GET_FLAGS(v4) & 0x01) == 0 && (p->IPv4PayloadSize >= udp_len))
{
// Calculate the checksum correctly based on the data in case of a non-fragmented packet
udp->Checksum = CalcChecksumForIPv4(v4->SrcIP, v4->DstIP, IP_PROTO_UDP, udp, udp_len, 0);
@ -2023,7 +2023,7 @@ void CorrectChecksum(PKT *p)
{
udp->Checksum = 0;
if (v6info->FragmentHeader == NULL || ((IPV6_GET_FLAGS(v6info->FragmentHeader) & IPV6_FRAGMENT_HEADER_FLAG_MORE_FRAGMENTS) == 0))
if ((v6info->FragmentHeader == NULL || ((IPV6_GET_FLAGS(v6info->FragmentHeader) & IPV6_FRAGMENT_HEADER_FLAG_MORE_FRAGMENTS) == 0)) && (v6info->PayloadSize >= udp_len))
{
// If the packet is not fragmented, recalculate the checksum
udp->Checksum = CalcChecksumForIPv6(&v6->SrcAddress, &v6->DestAddress, IP_PROTO_UDP, udp, udp_len, 0);
@ -2868,6 +2868,7 @@ PKT *ParsePacketIPv4WithDummyMacHeader(UCHAR *buf, UINT size)
{
UCHAR *tmp;
UINT tmp_size;
PKT *ret;
// Validate arguments
if (buf == NULL)
{
@ -2880,7 +2881,14 @@ PKT *ParsePacketIPv4WithDummyMacHeader(UCHAR *buf, UINT size)
WRITE_USHORT(tmp + 12, MAC_PROTO_IPV4);
Copy(tmp + 14, buf, size);
return ParsePacket(tmp, tmp_size);
ret = ParsePacket(tmp, tmp_size);
if (ret == NULL)
{
Free(tmp);
}
return ret;
}
// IPv4 parsing