From 6a6def775db00a88fa800ea4d08e6519539dacde Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 5 Jun 2020 10:01:19 +0100 Subject: [uri] Avoid appearing to access final byte of a potentially empty string The URI parsing code for "host[:port]" checks that the final character is not ']' in order to allow for IPv6 literals. If the entire "host[:port]" portion of the URL is an empty string, then this will access the preceding character. This does not result in accessing invalid memory (since the string is guaranteed by construction to always have a preceding character) and does not result in incorrect behaviour (since if the string is empty then strrchr() is guaranteed to return NULL), but it does make the code confusing to read. Fix by inverting the order of the two tests. Signed-off-by: Michael Brown --- src/core/uri.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core') diff --git a/src/core/uri.c b/src/core/uri.c index 73ad2b227..e9e512ab4 100644 --- a/src/core/uri.c +++ b/src/core/uri.c @@ -413,8 +413,8 @@ struct uri * parse_uri ( const char *uri_string ) { } /* Split host into host[:port] */ - if ( ( uri->host[ strlen ( uri->host ) - 1 ] != ']' ) && - ( tmp = strrchr ( uri->host, ':' ) ) ) { + if ( ( tmp = strrchr ( uri->host, ':' ) ) && + ( uri->host[ strlen ( uri->host ) - 1 ] != ']' ) ) { *(tmp++) = '\0'; uri->port = tmp; } -- cgit v1.2.3-55-g7522 From 8830f2f3514751e702199600fa6d0c42522a709a Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 19 Jun 2020 17:29:46 +0100 Subject: [parseopt] Treat empty integer strings in user input as invalid Signed-off-by: Michael Brown --- src/core/parseopt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/parseopt.c b/src/core/parseopt.c index 3ddf94f3d..007080088 100644 --- a/src/core/parseopt.c +++ b/src/core/parseopt.c @@ -93,7 +93,7 @@ int parse_integer ( char *text, unsigned int *value ) { /* Parse integer */ *value = strtoul ( text, &endp, 0 ); - if ( *endp ) { + if ( *endp || ( ! *text ) ) { printf ( "\"%s\": invalid integer value\n", text ); return -EINVAL_INTEGER; } -- cgit v1.2.3-55-g7522 From 2f032c84a2cbed37308da8b29e84efb6eb2fe9e5 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Tue, 7 Jul 2020 13:13:28 +0100 Subject: [libc] Provide an unoptimised generic_memcpy_reverse() Signed-off-by: Michael Brown --- src/core/string.c | 25 ++++++++++++++++++++----- src/include/string.h | 2 ++ 2 files changed, 22 insertions(+), 5 deletions(-) (limited to 'src/core') diff --git a/src/core/string.c b/src/core/string.c index 5bd9dae8b..c35015b59 100644 --- a/src/core/string.c +++ b/src/core/string.c @@ -52,7 +52,7 @@ void * generic_memset ( void *dest, int character, size_t len ) { } /** - * Copy memory region + * Copy memory region (forwards) * * @v dest Destination region * @v src Source region @@ -69,24 +69,39 @@ void * generic_memcpy ( void *dest, const void *src, size_t len ) { } /** - * Copy (possibly overlapping) memory region + * Copy memory region (backwards) * * @v dest Destination region * @v src Source region * @v len Length * @ret dest Destination region */ -void * generic_memmove ( void *dest, const void *src, size_t len ) { +void * generic_memcpy_reverse ( void *dest, const void *src, size_t len ) { const uint8_t *src_bytes = ( src + len ); uint8_t *dest_bytes = ( dest + len ); - if ( dest < src ) - return generic_memcpy ( dest, src, len ); while ( len-- ) *(--dest_bytes) = *(--src_bytes); return dest; } +/** + * Copy (possibly overlapping) memory region + * + * @v dest Destination region + * @v src Source region + * @v len Length + * @ret dest Destination region + */ +void * generic_memmove ( void *dest, const void *src, size_t len ) { + + if ( dest < src ) { + return generic_memcpy ( dest, src, len ); + } else { + return generic_memcpy_reverse ( dest, src, len ); + } +} + /** * Compare memory regions * diff --git a/src/include/string.h b/src/include/string.h index 0f4182001..5f5aecb92 100644 --- a/src/include/string.h +++ b/src/include/string.h @@ -15,6 +15,8 @@ extern void * generic_memset ( void *dest, int character, size_t len ) __nonnull; extern void * generic_memcpy ( void *dest, const void *src, size_t len ) __nonnull; +extern void * generic_memcpy_reverse ( void *dest, const void *src, + size_t len ) __nonnull; extern void * generic_memmove ( void *dest, const void *src, size_t len ) __nonnull; -- cgit v1.2.3-55-g7522 From a95a2eafc58b921431cbd53fc0cbff6a1a7761d1 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 15 Jul 2020 18:46:58 +0100 Subject: [xfer] Remove address family from definition of a socket opener All implemented socket openers provide definitions for both IPv4 and IPv6 using exactly the same opener method. Simplify the logic by omitting the address family from the definition. Signed-off-by: Michael Brown --- src/core/open.c | 4 +--- src/include/ipxe/open.h | 2 -- src/net/ping.c | 12 ++---------- src/net/tcp.c | 12 ++---------- src/net/udp.c | 12 ++---------- 5 files changed, 7 insertions(+), 35 deletions(-) (limited to 'src/core') diff --git a/src/core/open.c b/src/core/open.c index 9d665ffda..c27d8a021 100644 --- a/src/core/open.c +++ b/src/core/open.c @@ -147,10 +147,8 @@ int xfer_open_socket ( struct interface *intf, int semantics, socket_family_name ( peer->sa_family ) ); for_each_table_entry ( opener, SOCKET_OPENERS ) { - if ( ( opener->semantics == semantics ) && - ( opener->family == peer->sa_family ) ) { + if ( opener->semantics == semantics ) return opener->open ( intf, peer, local ); - } } DBGC ( INTF_COL ( intf ), "INTF " INTF_FMT " attempted to open " diff --git a/src/include/ipxe/open.h b/src/include/ipxe/open.h index 43d4cdc66..64e12d177 100644 --- a/src/include/ipxe/open.h +++ b/src/include/ipxe/open.h @@ -70,8 +70,6 @@ struct uri_opener { struct socket_opener { /** Communication semantics (e.g. SOCK_STREAM) */ int semantics; - /** Address family (e.g. AF_INET) */ - int family; /** Open socket * * @v intf Object interface diff --git a/src/net/ping.c b/src/net/ping.c index 3f4fa5c11..f0729e159 100644 --- a/src/net/ping.c +++ b/src/net/ping.c @@ -259,17 +259,9 @@ static int ping_open ( struct interface *xfer, struct sockaddr *peer, return rc; } -/** Ping IPv4 socket opener */ -struct socket_opener ping_ipv4_socket_opener __socket_opener = { +/** Ping socket opener */ +struct socket_opener ping_socket_opener __socket_opener = { .semantics = PING_SOCK_ECHO, - .family = AF_INET, - .open = ping_open, -}; - -/** Ping IPv6 socket opener */ -struct socket_opener ping_ipv6_socket_opener __socket_opener = { - .semantics = PING_SOCK_ECHO, - .family = AF_INET6, .open = ping_open, }; diff --git a/src/net/tcp.c b/src/net/tcp.c index 6bba44282..2a98221f6 100644 --- a/src/net/tcp.c +++ b/src/net/tcp.c @@ -1743,17 +1743,9 @@ static struct interface_descriptor tcp_xfer_desc = *************************************************************************** */ -/** TCP IPv4 socket opener */ -struct socket_opener tcp_ipv4_socket_opener __socket_opener = { +/** TCP socket opener */ +struct socket_opener tcp_socket_opener __socket_opener = { .semantics = TCP_SOCK_STREAM, - .family = AF_INET, - .open = tcp_open, -}; - -/** TCP IPv6 socket opener */ -struct socket_opener tcp_ipv6_socket_opener __socket_opener = { - .semantics = TCP_SOCK_STREAM, - .family = AF_INET6, .open = tcp_open, }; diff --git a/src/net/udp.c b/src/net/udp.c index 1fbc12d48..2c0b343dc 100644 --- a/src/net/udp.c +++ b/src/net/udp.c @@ -396,17 +396,9 @@ static struct interface_descriptor udp_xfer_desc = *************************************************************************** */ -/** UDP IPv4 socket opener */ -struct socket_opener udp_ipv4_socket_opener __socket_opener = { +/** UDP socket opener */ +struct socket_opener udp_socket_opener __socket_opener = { .semantics = UDP_SOCK_DGRAM, - .family = AF_INET, - .open = udp_open, -}; - -/** UDP IPv6 socket opener */ -struct socket_opener udp_ipv6_socket_opener __socket_opener = { - .semantics = UDP_SOCK_DGRAM, - .family = AF_INET6, .open = udp_open, }; -- cgit v1.2.3-55-g7522 From 0de5e601440bdb1f244b054384d6f77cefc86ba7 Mon Sep 17 00:00:00 2001 From: Michael J. Bazzinotti Date: Tue, 14 Jan 2020 13:15:02 -0500 Subject: [libc] Fix memcmp() to return proper values Fix memcmp() to return proper standard positive/negative values for unequal comparisons. Current implementation is backwards (i.e. the functions are returning negative when should be positive and vice-versa). Currently most consumers of these functions only check the return value for ==0 or !=0 and so we can safely change the implementation without breaking things. However, there is one call that checks the polarity of this function, and that is prf_sha1() for wireless WPA 4-way handshake. Due to the incorrect memcmp() polarity, the WPA handshake creates an incorrect PTK, and the handshake would fail after step 2. Undoubtedly, the AP noticed the supplicant failed the mic check. This commit fixes that issue. Similar to commit 3946aa9 ("[libc] Fix strcmp()/strncmp() to return proper values"). Signed-off-by: Michael Bazzinotti Modified-by: Michael Brown Signed-off-by: Michael Brown --- src/core/string.c | 2 +- src/tests/string_test.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/string.c b/src/core/string.c index c35015b59..188fe0864 100644 --- a/src/core/string.c +++ b/src/core/string.c @@ -116,7 +116,7 @@ int memcmp ( const void *first, const void *second, size_t len ) { int diff; while ( len-- ) { - diff = ( *(second_bytes++) - *(first_bytes++) ); + diff = ( *(first_bytes++) - *(second_bytes++) ); if ( diff ) return diff; } diff --git a/src/tests/string_test.c b/src/tests/string_test.c index a66501da3..88a730aec 100644 --- a/src/tests/string_test.c +++ b/src/tests/string_test.c @@ -109,6 +109,7 @@ static void string_test_exec ( void ) { ok ( memcmp ( "", "", 0 ) == 0 ); ok ( memcmp ( "Foo", "Foo", 3 ) == 0 ); ok ( memcmp ( "Foo", "Bar", 3 ) != 0 ); + ok ( memcmp ( "abc", "def", 3 ) < 0 ); /* Test strstr() */ { -- cgit v1.2.3-55-g7522