From f006fc9325443bd93d1429fd19d68109a2d73eea Mon Sep 17 00:00:00 2001 From: Olly Betts Date: Sat, 24 Mar 2018 11:10:11 +1300 Subject: [PATCH] Range check winsock2's socket() return value Winsock2's socket() returns the unsigned type SOCKET, which is a 32-bit type for WIN32 and a 64-bit type for WIN64. It seems we can always safely assign SOCKET to an int: failure is indicated by INVALID_SOCKET which will cast to -1 as an int, and it seems in practice that valid values all fit in 31-bits (and that we're not the only code to assume this since it makes it much easier to write code that deals with BSD sockets and winsock2's bastardised version of them) so Microsoft are unlikely to arbitrarily change that). But we should check and throw an exception rather than quietly mangling the value. See #738. --- xapian-core/common/safesysselect.h | 11 +++++------ xapian-core/common/safesyssocket.h | 39 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/xapian-core/common/safesysselect.h b/xapian-core/common/safesysselect.h index 476a8a2fe..92de5768d 100644 --- a/xapian-core/common/safesysselect.h +++ b/xapian-core/common/safesysselect.h @@ -1,7 +1,7 @@ /** @file safesysselect.h * @brief #include with portability workarounds. */ -/* Copyright (C) 2007,2011 Olly Betts +/* Copyright (C) 2007,2011,2018 Olly Betts * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -40,13 +40,12 @@ # include #else -// Under __WIN32__, socket() returns the unsigned type SOCKET. We can safely -// assign that to an int (it'll be a non-negative fd or INVALID_SOCKET, which -// will cast to -1 as an int), but when we use int in FD_SET, we get a warning -// about comparing signed and unsigned, so we add a wrapper to cast the fd -// argument of FD_SET to unsigned. // select() and FD_SET() are defined in : +// +// The FD_SET macro expects an unsigned type (SOCKET) for the fd and passing +// an int can result in a warning about comparing signed and unsigned, so we +// add a wrapper to cast the fd argument of FD_SET to unsigned. # include "safewinsock2.h" inline void xapian_FD_SET_(int fd, fd_set *set) { FD_SET(unsigned(fd), set); diff --git a/xapian-core/common/safesyssocket.h b/xapian-core/common/safesyssocket.h index 1af23879a..87b6b1246 100644 --- a/xapian-core/common/safesyssocket.h +++ b/xapian-core/common/safesyssocket.h @@ -1,7 +1,7 @@ /** @file safesyssocket.h * @brief #include with portability workarounds. */ -/* Copyright (C) 2012,2013,2014 Olly Betts +/* Copyright (C) 2012,2013,2014,2018 Olly Betts * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -29,7 +29,42 @@ # include "safewinsock2.h" #endif -#ifndef SOCK_CLOEXEC +#ifdef __WIN32__ +# include +# include "xapian/error.h" +# if defined SOCK_CLOEXEC +static_assert(!SOCK_CLOEXEC, "__WIN32__ doesn't support SOCK_CLOEXEC"); +# endif +# define SOCK_CLOEXEC 0 + +static_assert(std::is_unsigned::value, "SOCKET is unsigned"); + +inline int socket_(int domain, int type, int protocol) { + // Winsock2's socket() returns the unsigned type SOCKET, which is a 32-bit + // type for WIN32 and a 64-bit type for WIN64. + // + // It seems we can always safely assign SOCKET to an int: failure is indicated + // by INVALID_SOCKET which will cast to -1 as an int, and it seems in + // practice that valid values all fit in 31-bits (and that we're not the + // only code to assume this since it makes it much easier to write code + // that deals with BSD sockets and winsock2's bastardised version of them) + // so Microsoft are unlikely to arbitrarily change that). + // + // But we should check and throw an exception rather than quietly mangling + // the value. + SOCKET sock = socket(domain, type, protocol); + if (rare(sock > SOCKET(0x7fffffff) && sock != INVALID_SOCKET)) { + throw Xapian::NetworkError("socket() returned value > INT_MAX"); + } + return int(sock); +} + +# ifdef socket +# undef socket +# endif +# define socket(D,T,P) socket_(D,T,P) + +#elif !defined SOCK_CLOEXEC # define SOCK_CLOEXEC 0 #else // On Linux at least, sometimes SOCK_CLOEXEC is defined but the kernel doesn't -- 2.11.4.GIT