[PATCH 0/2] MR5842: ws2_32/tests: Test fromlen for recvfrom().

[Zhiyi Zhang (＠zhiyi)]

Test that specifying a too small or too large fromlen for recvfrom() shouldn't write unnecessary data.

[PATCH 1/2] ws2_32/tests: Test fromlen for recvfrom().

[Zhiyi Zhang]

From: Zhiyi Zhang

Test that specifying a too small or too large fromlen for recvfrom() shouldn't write unnecessary data.
---
 dlls/ws2_32/tests/sock.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c
index 6954b3e077a..57f1e6f9f97 100644
--- a/dlls/ws2_32/tests/sock.c
+++ b/dlls/ws2_32/tests/sock.c
@@ -3185,7 +3185,7 @@ static void test_UDP(void)
     /* peer 0 receives data from all other peers */
     static const TIMEVAL timeout_zero = {0};
     struct sock_info peer[NUM_UDP_PEERS];
-    char buf[16];
+    char buf[16], sockaddr_buf[1024];
     int ss, i, n_recv, n_sent, ret;
     struct sockaddr_in addr;
     int sock;
@@ -3215,11 +3215,40 @@ static void test_UDP(void)
         ok ( peer[i].addr.sin_port != htons ( 0 ), "UDP: bind() did not associate port\n" );
     }

+    /* Test that specifying a too small fromlen for recvfrom() shouldn't write unnecessary data */
+    n_sent = sendto ( peer[1].s, buf, sizeof(buf), 0, (struct sockaddr *)&peer[0].addr, sizeof(peer[0].addr) );
+    ok ( n_sent == sizeof(buf), "UDP: sendto() sent wrong amount of data or socket error: %d\n", n_sent );
+
+    sockaddr_buf[0] = 'A';
+    ss = 1;
+    n_recv = recvfrom ( peer[0].s, buf, sizeof(buf), 0, (struct sockaddr *)sockaddr_buf, &ss );
+    todo_wine
+    ok ( n_recv == SOCKET_ERROR, "UDP: recvfrom() succeeded\n" );
+    todo_wine
+    ok ( sockaddr_buf[0] == 'A', "UDP: marker got overwritten\n" );
+    if ( n_recv == SOCKET_ERROR )
+    {
+        ss = sizeof ( peer[0].addr );
+        n_recv = recvfrom ( peer[0].s, buf, sizeof(buf), 0, (struct sockaddr *)sockaddr_buf, &ss );
+        ok ( n_recv == sizeof(buf), "UDP: recvfrom() failed\n" );
+    }
+
+    /* Test that specifying a large fromlen for recvfrom() shouldn't write unnecessary data besides the socket address */
+    n_sent = sendto ( peer[1].s, buf, sizeof(buf), 0, (struct sockaddr *)&peer[0].addr, sizeof(peer[0].addr) );
+    ok ( n_sent == sizeof(buf), "UDP: sendto() sent wrong amount of data or socket error: %d\n", n_sent );
+
+    sockaddr_buf[1023] = 'B';
+    ss = sizeof(sockaddr_buf);
+    n_recv = recvfrom ( peer[0].s, buf, sizeof(buf), 0, (struct sockaddr *)sockaddr_buf, &ss );
+    ok ( n_recv == sizeof(buf), "UDP: recvfrom() received wrong amount of data or socket error: %d\n", n_recv );
+    todo_wine
+    ok ( sockaddr_buf[1023] == 'B', "UDP: marker got overwritten\n" );
+
     /* test getsockname() */
     ok ( peer[0].addr.sin_port == htons ( SERVERPORT ), "UDP: getsockname returned incorrect peer port\n" );

for ( i = 1; i < NUM_UDP_PEERS; i++ ) {
-        /* send client's ip */
+        /* send client's port */
         memcpy( buf, &peer[i].addr.sin_port, sizeof(peer[i].addr.sin_port) );
         n_sent = sendto ( peer[i].s, buf, sizeof(buf), 0, (struct sockaddr*) &peer[0].addr, sizeof(peer[0].addr) );
         ok ( n_sent == sizeof(buf), "UDP: sendto() sent wrong amount of data or socket error: %d\n", n_sent );

[PATCH 2/2] ntdll: Don't zero out socket address in sockaddr_from_unix().

[Zhiyi Zhang]

From: Zhiyi Zhang

tallygatewayserver.exe specifies a from sockaddr pointing to a heap buffer smaller than 128 bytes
yet it passes 128 as the fromlen to recvfrom(). So the memset(wsaddr, 0, wsaddrlen) call in
sockaddr_from_unix() ends up trashing other data in the heap, causing the application to crash.
Although this is an application bug, tests on Windows also showed that the socket address buffer
should be written only with the necessary socket address data, thus preventing the crash.
---
 dlls/ntdll/unix/socket.c | 2 --
 dlls/ws2_32/tests/sock.c | 2 --
 2 files changed, 4 deletions(-)

diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c
index 65f30759db3..ea2f5d3a670 100644
--- a/dlls/ntdll/unix/socket.c
+++ b/dlls/ntdll/unix/socket.c
@@ -298,8 +298,6 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl

static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_sockaddr *wsaddr, socklen_t wsaddrlen )
 {
-    memset( wsaddr, 0, wsaddrlen );
-
     switch (uaddr->addr.sa_family)
     {
     case AF_INET:
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c
index 57f1e6f9f97..e8c618cc0af 100644
--- a/dlls/ws2_32/tests/sock.c
+++ b/dlls/ws2_32/tests/sock.c
@@ -3224,7 +3224,6 @@ static void test_UDP(void)
     n_recv = recvfrom ( peer[0].s, buf, sizeof(buf), 0, (struct sockaddr *)sockaddr_buf, &ss );
     todo_wine
     ok ( n_recv == SOCKET_ERROR, "UDP: recvfrom() succeeded\n" );
-    todo_wine
     ok ( sockaddr_buf[0] == 'A', "UDP: marker got overwritten\n" );
     if ( n_recv == SOCKET_ERROR )
     {
@@ -3241,7 +3240,6 @@ static void test_UDP(void)
     ss = sizeof(sockaddr_buf);
     n_recv = recvfrom ( peer[0].s, buf, sizeof(buf), 0, (struct sockaddr *)sockaddr_buf, &ss );
     ok ( n_recv == sizeof(buf), "UDP: recvfrom() received wrong amount of data or socket error: %d\n", n_recv );
-    todo_wine
     ok ( sockaddr_buf[1023] == 'B', "UDP: marker got overwritten\n" );

/* test getsockname() */

[Elizabeth Figura (＠zfigura)]

Yeah, I have no idea why I wrote that :x

[PATCH 0/2] MR5842: ws2_32/tests: Test fromlen for recvfrom(). - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.