[PATCH 1/5] server: Validate the output size of IOCTL_AFD_BIND against the input address size rather than sizeof(struct sockaddr).
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/ws2_32/tests/afd.c | 4 ++-- server/sock.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index 32daab0beab..c31d1def139 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -1610,7 +1610,7 @@ static void test_bind(void) memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, params6_size, &addr6, sizeof(addr6) - 1); - todo_wine ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret); + ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, @@ -1620,7 +1620,7 @@ static void test_bind(void) memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, params6_size, &addr6, sizeof(struct sockaddr_in6_old)); - todo_wine ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret); + ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); memset(&io, 0xcc, sizeof(io)); diff --git a/server/sock.c b/server/sock.c index f730bb31c55..cce91f5150d 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2492,7 +2492,7 @@ static int sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) } in_size = get_req_data_size() - get_reply_max_size(); if (in_size < offsetof(struct afd_bind_params, addr.sa_data) - || get_reply_max_size() < sizeof(struct WS_sockaddr)) + || get_reply_max_size() < in_size - sizeof(int)) { set_error( STATUS_INVALID_PARAMETER ); return 0;
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/ws2_32/tests/afd.c | 29 +++++++++++++---------------- server/sock.c | 23 ++++++----------------- 2 files changed, 19 insertions(+), 33 deletions(-)
diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index c31d1def139..50922500859 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -1582,17 +1582,17 @@ static void test_bind(void) memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, params6_size - 1, &addr6, sizeof(addr6)); - todo_wine ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret); + ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, offsetof(struct afd_bind_params, addr) + sizeof(struct sockaddr_in6_old), &addr6, sizeof(addr6)); - todo_wine ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret); + ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, offsetof(struct afd_bind_params, addr.sa_data), &addr6, sizeof(addr6)); - todo_wine ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret); + ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, @@ -1604,8 +1604,8 @@ static void test_bind(void) params, params6_size, &addr6, sizeof(addr6)); todo_wine ok(ret == STATUS_PENDING, "got %#x\n", ret); ret = WaitForSingleObject(event, 0); - todo_wine ok(!ret, "got %#x\n", ret); - todo_wine ok(io.Status == STATUS_INVALID_ADDRESS_COMPONENT, "got %#x\n", io.Status); + ok(!ret, "got %#x\n", ret); + ok(io.Status == STATUS_INVALID_ADDRESS_COMPONENT, "got %#x\n", io.Status);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, @@ -1615,7 +1615,7 @@ static void test_bind(void) memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, params6_size - 1, &addr6, sizeof(addr6) - 1); - todo_wine ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret); + ok(ret == STATUS_INVALID_ADDRESS, "got %#x\n", ret);
memcpy(¶ms->addr, &bind_addr6, sizeof(bind_addr6)); ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, @@ -1629,15 +1629,12 @@ static void test_bind(void) params, params6_size, &addr6, sizeof(addr6)); todo_wine ok(ret == STATUS_PENDING, "got %#x\n", ret); ret = WaitForSingleObject(event, 0); - todo_wine - { - ok(!ret, "got %#x\n", ret); - ok(!io.Status, "got %#x\n", io.Status); - ok(io.Information == sizeof(addr6), "got %#Ix\n", io.Information); - ok(addr6.sin6_family == AF_INET6, "got family %u\n", addr6.sin6_family); - ok(!memcmp(&addr6.sin6_addr, &bind_addr6.sin6_addr, sizeof(addr6.sin6_addr)), "address didn't match\n"); - ok(!addr6.sin6_flowinfo, "got flow info %#x\n", addr6.sin6_flowinfo); - } + ok(!ret, "got %#x\n", ret); + ok(!io.Status, "got %#x\n", io.Status); + ok(io.Information == sizeof(addr6), "got %#Ix\n", io.Information); + ok(addr6.sin6_family == AF_INET6, "got family %u\n", addr6.sin6_family); + ok(!memcmp(&addr6.sin6_addr, &bind_addr6.sin6_addr, sizeof(addr6.sin6_addr)), "address didn't match\n"); + ok(!addr6.sin6_flowinfo, "got flow info %#x\n", addr6.sin6_flowinfo); ok(addr6.sin6_port, "expected nonzero port\n");
/* getsockname() returns EINVAL here. Possibly the socket name is cached (in shared memory?) */ @@ -1645,7 +1642,7 @@ static void test_bind(void) ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_GETSOCKNAME, NULL, 0, &addr6_2, sizeof(addr6_2)); ok(!ret, "got %#x\n", ret); - todo_wine ok(!memcmp(&addr6, &addr6_2, sizeof(addr6)), "addresses didn't match\n"); + ok(!memcmp(&addr6, &addr6_2, sizeof(addr6)), "addresses didn't match\n");
ret = NtDeviceIoControlFile((HANDLE)s, event, NULL, NULL, &io, IOCTL_AFD_BIND, params, params6_size, &addr6, sizeof(addr6)); diff --git a/server/sock.c b/server/sock.c index cce91f5150d..cd77ff7bdca 100644 --- a/server/sock.c +++ b/server/sock.c @@ -311,7 +311,7 @@ static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_socka { struct WS_sockaddr_in6 win = {0};
- if (wsaddrlen < sizeof(struct WS_sockaddr_in6_old)) return -1; + if (wsaddrlen < sizeof(win)) return -1; win.sin6_family = WS_AF_INET6; win.sin6_port = uaddr->in6.sin6_port; win.sin6_flowinfo = uaddr->in6.sin6_flowinfo; @@ -319,13 +319,8 @@ static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_socka #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID win.sin6_scope_id = uaddr->in6.sin6_scope_id; #endif - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) - { - memcpy( wsaddr, &win, sizeof(struct WS_sockaddr_in6) ); - return sizeof(struct WS_sockaddr_in6); - } - memcpy( wsaddr, &win, sizeof(struct WS_sockaddr_in6_old) ); - return sizeof(struct WS_sockaddr_in6_old); + memcpy( wsaddr, &win, sizeof(win) ); + return sizeof(win); }
#ifdef HAS_IPX @@ -391,19 +386,14 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl { struct WS_sockaddr_in6 win = {0};
- if (wsaddrlen < sizeof(struct WS_sockaddr_in6_old)) return 0; - if (wsaddrlen < sizeof(struct WS_sockaddr_in6)) - memcpy( &win, wsaddr, sizeof(struct WS_sockaddr_in6_old) ); - else - memcpy( &win, wsaddr, sizeof(struct WS_sockaddr_in6) ); - + if (wsaddrlen < sizeof(win)) return 0; + memcpy( &win, wsaddr, sizeof(win) ); uaddr->in6.sin6_family = AF_INET6; uaddr->in6.sin6_port = win.sin6_port; uaddr->in6.sin6_flowinfo = win.sin6_flowinfo; memcpy( &uaddr->in6.sin6_addr, &win.sin6_addr, sizeof(win.sin6_addr) ); #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) - uaddr->in6.sin6_scope_id = win.sin6_scope_id; + uaddr->in6.sin6_scope_id = win.sin6_scope_id; #endif return sizeof(uaddr->in6); } @@ -462,7 +452,6 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl #endif
case sizeof(struct WS_sockaddr_in6): - case sizeof(struct WS_sockaddr_in6_old): return sizeof(uaddr->in6); }
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/ntdll/unix/socket.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index c38f4f53089..6b863fd61f0 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -232,19 +232,14 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl { struct WS_sockaddr_in6 win = {0};
- if (wsaddrlen < sizeof(struct WS_sockaddr_in6_old)) return 0; - if (wsaddrlen < sizeof(struct WS_sockaddr_in6)) - memcpy( &win, wsaddr, sizeof(struct WS_sockaddr_in6_old) ); - else - memcpy( &win, wsaddr, sizeof(struct WS_sockaddr_in6) ); - + if (wsaddrlen < sizeof(win)) return 0; + memcpy( &win, wsaddr, sizeof(win) ); uaddr->in6.sin6_family = AF_INET6; uaddr->in6.sin6_port = win.sin6_port; uaddr->in6.sin6_flowinfo = win.sin6_flowinfo; memcpy( &uaddr->in6.sin6_addr, &win.sin6_addr, sizeof(win.sin6_addr) ); #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) - uaddr->in6.sin6_scope_id = win.sin6_scope_id; + uaddr->in6.sin6_scope_id = win.sin6_scope_id; #endif return sizeof(uaddr->in6); } @@ -303,7 +298,6 @@ static socklen_t sockaddr_to_unix( const struct WS_sockaddr *wsaddr, int wsaddrl #endif
case sizeof(struct WS_sockaddr_in6): - case sizeof(struct WS_sockaddr_in6_old): return sizeof(uaddr->in6); }
@@ -335,7 +329,7 @@ static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_socka { struct WS_sockaddr_in6 win = {0};
- if (wsaddrlen < sizeof(struct WS_sockaddr_in6_old)) return -1; + if (wsaddrlen < sizeof(win)) return -1; win.sin6_family = WS_AF_INET6; win.sin6_port = uaddr->in6.sin6_port; win.sin6_flowinfo = uaddr->in6.sin6_flowinfo; @@ -343,13 +337,8 @@ static int sockaddr_from_unix( const union unix_sockaddr *uaddr, struct WS_socka #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID win.sin6_scope_id = uaddr->in6.sin6_scope_id; #endif - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) - { - memcpy( wsaddr, &win, sizeof(struct WS_sockaddr_in6) ); - return sizeof(struct WS_sockaddr_in6); - } - memcpy( wsaddr, &win, sizeof(struct WS_sockaddr_in6_old) ); - return sizeof(struct WS_sockaddr_in6_old); + memcpy( wsaddr, &win, sizeof(win) ); + return sizeof(win); }
#ifdef HAS_IPX
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/ws2_32/socket.c | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index b4786e54b38..16bece395b8 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1097,14 +1097,14 @@ unsigned int ws_sockaddr_ws2u( const struct WS_sockaddr *wsaddr, int wsaddrlen, /* Note: Windows has 2 versions of the sockaddr_in6 struct, one with * scope_id, one without. */ - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6_old)) { + if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) { uaddrlen = sizeof(struct sockaddr_in6); memset( uaddr, 0, uaddrlen ); uin6->sin6_family = AF_INET6; uin6->sin6_port = win6->sin6_port; uin6->sin6_flowinfo = win6->sin6_flowinfo; #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID - if (wsaddrlen >= sizeof(struct WS_sockaddr_in6)) uin6->sin6_scope_id = win6->sin6_scope_id; + uin6->sin6_scope_id = win6->sin6_scope_id; #endif memcpy(&uin6->sin6_addr,&win6->sin6_addr,16); /* 16 bytes = 128 address bits */ break; @@ -1299,25 +1299,20 @@ int ws_sockaddr_u2ws(const struct sockaddr *uaddr, struct WS_sockaddr *wsaddr, i #endif case AF_INET6: { const struct sockaddr_in6* uin6 = (const struct sockaddr_in6*)uaddr; - struct WS_sockaddr_in6_old* win6old = (struct WS_sockaddr_in6_old*)wsaddr; + struct WS_sockaddr_in6 *win6 = (struct WS_sockaddr_in6 *)wsaddr;
- if (*wsaddrlen < sizeof(struct WS_sockaddr_in6_old)) + if (*wsaddrlen < sizeof(struct WS_sockaddr_in6)) return -1; - win6old->sin6_family = WS_AF_INET6; - win6old->sin6_port = uin6->sin6_port; - win6old->sin6_flowinfo = uin6->sin6_flowinfo; - memcpy(&win6old->sin6_addr,&uin6->sin6_addr,16); /* 16 bytes = 128 address bits */ + win6->sin6_family = WS_AF_INET6; + win6->sin6_port = uin6->sin6_port; + win6->sin6_flowinfo = uin6->sin6_flowinfo; + memcpy(&win6->sin6_addr, &uin6->sin6_addr, 16); /* 16 bytes = 128 address bits */ #ifdef HAVE_STRUCT_SOCKADDR_IN6_SIN6_SCOPE_ID - if (*wsaddrlen >= sizeof(struct WS_sockaddr_in6)) { - struct WS_sockaddr_in6* win6 = (struct WS_sockaddr_in6*)wsaddr; - win6->sin6_scope_id = uin6->sin6_scope_id; - *wsaddrlen = sizeof(struct WS_sockaddr_in6); - } - else - *wsaddrlen = sizeof(struct WS_sockaddr_in6_old); + win6->sin6_scope_id = uin6->sin6_scope_id; #else - *wsaddrlen = sizeof(struct WS_sockaddr_in6_old); + win6->sin6_scope_id = 0; #endif + *wsaddrlen = sizeof(struct WS_sockaddr_in6); return 0; } case AF_INET: {
Signed-off-by: Zebediah Figura zfigura@codeweavers.com --- dlls/ws2_32/socket.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 16bece395b8..7fb48698d2a 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -192,8 +192,8 @@ const char *debugstr_sockaddr( const struct WS_sockaddr *a ) p = WS_inet_ntop( WS_AF_INET6, &sin->sin6_addr, buf, sizeof(buf) ); if (!p) p = "(unknown IPv6 address)"; - return wine_dbg_sprintf("{ family AF_INET6, address %s, port %d }", - p, ntohs(sin->sin6_port)); + return wine_dbg_sprintf("{ family AF_INET6, address %s, flow label %#x, port %d, scope %u }", + p, sin->sin6_flowinfo, ntohs(sin->sin6_port), sin->sin6_scope_id ); } case WS_AF_IPX: {
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=94482
Your paranoid android.
=== debiant2 (32 bit WoW report) ===
Report validation errors: sock: Timeout
=== debiant2 (build log) ===
WineRunWineTest.pl:error: The task timed out
-
Marvin -
Zebediah Figura