[PATCH 3/3] Add _FORTIFY_SOURCE support for inet_ntop

[Florian Weimer]

* Frédéric Bérat:

> - Create the __inet_ntop_chk routine that verifies that the builtin size
> of the destination buffer is at least as big as the size given by the
> user.
> - Redirect calls from inet_ntop to __inet_ntop_chk or __inet_ntop_warn
> - Update the abilist for this new routine
> - Update man pages to mention the new fortification

Update [the manual] to mention the new fortification

(Manpages are a separate project.)

> diff --git a/debug/tst-fortify.c b/debug/tst-fortify.c
> index f8ccc2dff5..128f271c03 100644
> --- a/debug/tst-fortify.c
> +++ b/debug/tst-fortify.c
> @@ -23,6 +23,7 @@
>  
>  #include <assert.h>
>  #include <fcntl.h>
> +#include <arpa/inet.h>
>  #include <limits.h>
>  #include <locale.h>
>  #include <obstack.h>
> @@ -1832,6 +1833,26 @@ do_test (void)
>  # endif
>  #endif
>  
> +  struct in6_addr addr6;
> +  struct in_addr addr;
> +  char addrstr6[INET6_ADDRSTRLEN];
> +  char addrstr[INET_ADDRSTRLEN];
> +
> +  if (inet_ntop (AF_INET6, &addr6, addrstr6, sizeof (addrstr6)) == NULL)
> +    FAIL ();
> +  if (inet_ntop (AF_INET, &addr, addrstr, sizeof (addrstr)) == NULL)
> +    FAIL ();

Doesn't this use uninitialized inputs?  Maybe add an initialization to
addr, addr6.

> +
> +#if __USE_FORTIFY_LEVEL >= 1
> +  CHK_FAIL_START
> +  inet_ntop (AF_INET6, &addr6, buf, INET6_ADDRSTRLEN);
> +  CHK_FAIL_END
> +
> +  CHK_FAIL_START
> +  inet_ntop (AF_INET, &addr, buf, INET_ADDRSTRLEN);
> +  CHK_FAIL_END
> +#endif

(buf is just 10 bytes, so both INET6_ADDRSTRLEN and INET_ADDRSTRLEN will
trap.)

> diff --git a/include/arpa/inet.h b/include/arpa/inet.h
> index f389c28cb2..d8afbf259b 100644
> --- a/include/arpa/inet.h
> +++ b/include/arpa/inet.h
> @@ -3,12 +3,16 @@
>  #include <inet/arpa/inet.h>
>  
>  #ifndef _ISOMAC
> +/* Declare functions with security checks.  */
> +#include <bits/inet-fortified-decl.h>

Why is this necessary?  Wouldn't this included by <inet/arpa/inet.h>
above?  Is this to check that the declaration matches the definition?

> diff --git a/inet/arpa/inet.h b/inet/arpa/inet.h
> index 42d38c330d..d381df7066 100644
> --- a/inet/arpa/inet.h
> +++ b/inet/arpa/inet.h
> @@ -65,7 +65,6 @@ extern const char *inet_ntop (int __af, const void *__restrict __cp,
>  			      char *__restrict __buf, socklen_t __len)
>       __THROW;
>  
> -

Spurious whitespace change.

> diff --git a/inet/bits/inet-fortified.h b/inet/bits/inet-fortified.h
> new file mode 100644
> index 0000000000..86b6d56d7b
> --- /dev/null
> +++ b/inet/bits/inet-fortified.h

> +__fortify_function __attribute_overloadable__ const char *
> +__NTH (inet_ntop (int __af,
> +	   __fortify_clang_overload_arg (const void *, __restrict, __src),
> +	   char *__restrict __dst, socklen_t __dst_size))
> +    __fortify_clang_warning_only_if_bos_lt(__dst_size, __dst,
> +					   "inet_ntop called with bigger length "
> +					   "than size of destination buffer")
> +{
> +  return __glibc_fortify (inet_ntop, __dst_size, sizeof(char),
> +			  __glibc_objsize(__dst),
> +			  __af, __src, __dst, __dst_size);

Missing whitespace before various '('.

Thanks,
Florian