diff mbox

Fix unbound stack use in NIS NSS module

Message ID mvmegzzxwat.fsf@hawking.suse.de
State Committed
Headers

Commit Message

Andreas Schwab May 12, 2014, 7:58 a.m. UTC 
  yp_match needs to put its request in a single RPC packet, so don't
bother trying to support big items.

Andreas.

	[BZ #16932]
	* nis/nss_nis/nis-hosts.c (internal_gethostbyname2_r)
	(_nss_nis_gethostbyname4_r): Return error if item length is larger
	than maximum RPC packet size.
	* nis/nss_nis/nis-initgroups.c (initgroups_netid): Likewise.
	* nis/nss_nis/nis-network.c (_nss_nis_getnetbyname_r): Likewise.
	* nis/nss_nis/nis-service.c (_nss_nis_getservbyname_r)
	(_nss_nis_getservbyport_r): Likewise.
---
 nis/nss_nis/nis-hosts.c      | 14 ++++++++++++++
 nis/nss_nis/nis-initgroups.c |  7 +++++++
 nis/nss_nis/nis-network.c    |  7 +++++++
 nis/nss_nis/nis-service.c    | 14 ++++++++++++++
 4 files changed, 42 insertions(+)

Comments

Ondrej Bilka May 12, 2014, 12:09 p.m. UTC | #1 
On Mon, May 12, 2014 at 09:58:50AM +0200, Andreas Schwab wrote:
> yp_match needs to put its request in a single RPC packet, so don't
> bother trying to support big items.
> 
> Andreas.
> 
> 	[BZ #16932]
> 	* nis/nss_nis/nis-hosts.c (internal_gethostbyname2_r)
> 	(_nss_nis_gethostbyname4_r): Return error if item length is larger
> 	than maximum RPC packet size.
> 	* nis/nss_nis/nis-initgroups.c (initgroups_netid): Likewise.
> 	* nis/nss_nis/nis-network.c (_nss_nis_getnetbyname_r): Likewise.
> 	* nis/nss_nis/nis-service.c (_nss_nis_getservbyname_r)
> 	(_nss_nis_getservbyport_r): Likewise.
> ---

A rationale of this patch is to prevent buffer overflow of subsequent
stack allocation.

This duplicates code a bit but I did not came with better solution so I am ok with that.
Patchwork Bot May 12, 2014, 1:34 p.m. UTC | #2 
On 12 May 2014 13:28, Andreas Schwab <schwab@suse.de> wrote:
> yp_match needs to put its request in a single RPC packet, so don't
> bother trying to support big items.

Won't this apply to netgroups too?

Siddhesh

>
> Andreas.
>
>         [BZ #16932]
>         * nis/nss_nis/nis-hosts.c (internal_gethostbyname2_r)
>         (_nss_nis_gethostbyname4_r): Return error if item length is larger
>         than maximum RPC packet size.
>         * nis/nss_nis/nis-initgroups.c (initgroups_netid): Likewise.
>         * nis/nss_nis/nis-network.c (_nss_nis_getnetbyname_r): Likewise.
>         * nis/nss_nis/nis-service.c (_nss_nis_getservbyname_r)
>         (_nss_nis_getservbyport_r): Likewise.
> ---
>  nis/nss_nis/nis-hosts.c      | 14 ++++++++++++++
>  nis/nss_nis/nis-initgroups.c |  7 +++++++
>  nis/nss_nis/nis-network.c    |  7 +++++++
>  nis/nss_nis/nis-service.c    | 14 ++++++++++++++
>  4 files changed, 42 insertions(+)
>
> diff --git a/nis/nss_nis/nis-hosts.c b/nis/nss_nis/nis-hosts.c
> index 462176e..d6192b1 100644
> --- a/nis/nss_nis/nis-hosts.c
> +++ b/nis/nss_nis/nis-hosts.c
> @@ -270,6 +270,13 @@ internal_gethostbyname2_r (const char *name, int af, struct hostent *host,
>
>    /* Convert name to lowercase.  */
>    size_t namlen = strlen (name);
> +  /* Limit name length to the maximum size of an RPC packet.  */
> +  if (namlen > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    char name2[namlen + 1];
>    size_t i;
>
> @@ -461,6 +468,13 @@ _nss_nis_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
>
>    /* Convert name to lowercase.  */
>    size_t namlen = strlen (name);
> +  /* Limit name length to the maximum size of an RPC packet.  */
> +  if (namlen > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    char name2[namlen + 1];
>    size_t i;
>
> diff --git a/nis/nss_nis/nis-initgroups.c b/nis/nss_nis/nis-initgroups.c
> index e8fcca1..9542fae 100644
> --- a/nis/nss_nis/nis-initgroups.c
> +++ b/nis/nss_nis/nis-initgroups.c
> @@ -150,6 +150,13 @@ initgroups_netid (uid_t uid, gid_t group, long int *start, long int *size,
>                   gid_t **groupsp, long int limit, int *errnop,
>                   const char *domainname)
>  {
> +  /* Limit domainname length to the maximum size of an RPC packet.  */
> +  if (strlen (domainname) > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    /* Prepare the key.  The form is "unix.UID@DOMAIN" with the UID and
>       DOMAIN field filled in appropriately.  */
>    char key[sizeof ("unix.@") + sizeof (uid_t) * 3 + strlen (domainname)];
> diff --git a/nis/nss_nis/nis-network.c b/nis/nss_nis/nis-network.c
> index f28fbda..f1b72bc 100644
> --- a/nis/nss_nis/nis-network.c
> +++ b/nis/nss_nis/nis-network.c
> @@ -179,6 +179,13 @@ _nss_nis_getnetbyname_r (const char *name, struct netent *net, char *buffer,
>
>    /* Convert name to lowercase.  */
>    size_t namlen = strlen (name);
> +  /* Limit name length to the maximum size of an RPC packet.  */
> +  if (namlen > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    char name2[namlen + 1];
>    size_t i;
>
> diff --git a/nis/nss_nis/nis-service.c b/nis/nss_nis/nis-service.c
> index f9b4a86..44e4e13 100644
> --- a/nis/nss_nis/nis-service.c
> +++ b/nis/nss_nis/nis-service.c
> @@ -271,6 +271,13 @@ _nss_nis_getservbyname_r (const char *name, const char *protocol,
>    /* If the protocol is given, we could try if our NIS server knows
>       about services.byservicename map. If yes, we only need one query.  */
>    size_t keylen = strlen (name) + (protocol ? 1 + strlen (protocol) : 0);
> +  /* Limit key length to the maximum size of an RPC packet.  */
> +  if (keylen > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    char key[keylen + 1];
>
>    /* key is: "name/proto" */
> @@ -355,6 +362,13 @@ _nss_nis_getservbyport_r (int port, const char *protocol,
>       Otherwise try first port/tcp, then port/udp and then fallback
>       to sequential scanning of services.byname.  */
>    const char *proto = protocol != NULL ? protocol : "tcp";
> +  /* Limit protocol name length to the maximum size of an RPC packet.  */
> +  if (strlen (proto) > UDPMSGSIZE)
> +    {
> +      *errnop = ERANGE;
> +      return NSS_STATUS_UNAVAIL;
> +    }
> +
>    do
>      {
>        /* key is: "port/proto" */
> --
> 1.9.2
>
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
Andreas Schwab May 12, 2014, 1:57 p.m. UTC | #3 
Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> writes:

> On 12 May 2014 13:28, Andreas Schwab <schwab@suse.de> wrote:
>> yp_match needs to put its request in a single RPC packet, so don't
>> bother trying to support big items.
>
> Won't this apply to netgroups too?

In which way?

Andreas.
Patchwork Bot May 12, 2014, 2:12 p.m. UTC | #4 
On 12 May 2014 19:27, Andreas Schwab <schwab@suse.de> wrote:
> Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> writes:
>
>> On 12 May 2014 13:28, Andreas Schwab <schwab@suse.de> wrote:
>>> yp_match needs to put its request in a single RPC packet, so don't
>>> bother trying to support big items.
>>
>> Won't this apply to netgroups too?
>
> In which way?

In no way at all; I didn't read the patch (or the problem) carefully.
Sorry for the noise.

Siddhesh
diff mbox

Patch

diff --git a/nis/nss_nis/nis-hosts.c b/nis/nss_nis/nis-hosts.c
index 462176e..d6192b1 100644
--- a/nis/nss_nis/nis-hosts.c
+++ b/nis/nss_nis/nis-hosts.c
@@ -270,6 +270,13 @@  internal_gethostbyname2_r (const char *name, int af, struct hostent *host,
 
   /* Convert name to lowercase.  */
   size_t namlen = strlen (name);
+  /* Limit name length to the maximum size of an RPC packet.  */
+  if (namlen > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   char name2[namlen + 1];
   size_t i;
 
@@ -461,6 +468,13 @@  _nss_nis_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
 
   /* Convert name to lowercase.  */
   size_t namlen = strlen (name);
+  /* Limit name length to the maximum size of an RPC packet.  */
+  if (namlen > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   char name2[namlen + 1];
   size_t i;
 
diff --git a/nis/nss_nis/nis-initgroups.c b/nis/nss_nis/nis-initgroups.c
index e8fcca1..9542fae 100644
--- a/nis/nss_nis/nis-initgroups.c
+++ b/nis/nss_nis/nis-initgroups.c
@@ -150,6 +150,13 @@  initgroups_netid (uid_t uid, gid_t group, long int *start, long int *size,
 		  gid_t **groupsp, long int limit, int *errnop,
 		  const char *domainname)
 {
+  /* Limit domainname length to the maximum size of an RPC packet.  */
+  if (strlen (domainname) > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   /* Prepare the key.  The form is "unix.UID@DOMAIN" with the UID and
      DOMAIN field filled in appropriately.  */
   char key[sizeof ("unix.@") + sizeof (uid_t) * 3 + strlen (domainname)];
diff --git a/nis/nss_nis/nis-network.c b/nis/nss_nis/nis-network.c
index f28fbda..f1b72bc 100644
--- a/nis/nss_nis/nis-network.c
+++ b/nis/nss_nis/nis-network.c
@@ -179,6 +179,13 @@  _nss_nis_getnetbyname_r (const char *name, struct netent *net, char *buffer,
 
   /* Convert name to lowercase.  */
   size_t namlen = strlen (name);
+  /* Limit name length to the maximum size of an RPC packet.  */
+  if (namlen > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   char name2[namlen + 1];
   size_t i;
 
diff --git a/nis/nss_nis/nis-service.c b/nis/nss_nis/nis-service.c
index f9b4a86..44e4e13 100644
--- a/nis/nss_nis/nis-service.c
+++ b/nis/nss_nis/nis-service.c
@@ -271,6 +271,13 @@  _nss_nis_getservbyname_r (const char *name, const char *protocol,
   /* If the protocol is given, we could try if our NIS server knows
      about services.byservicename map. If yes, we only need one query.  */
   size_t keylen = strlen (name) + (protocol ? 1 + strlen (protocol) : 0);
+  /* Limit key length to the maximum size of an RPC packet.  */
+  if (keylen > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   char key[keylen + 1];
 
   /* key is: "name/proto" */
@@ -355,6 +362,13 @@  _nss_nis_getservbyport_r (int port, const char *protocol,
      Otherwise try first port/tcp, then port/udp and then fallback
      to sequential scanning of services.byname.  */
   const char *proto = protocol != NULL ? protocol : "tcp";
+  /* Limit protocol name length to the maximum size of an RPC packet.  */
+  if (strlen (proto) > UDPMSGSIZE)
+    {
+      *errnop = ERANGE;
+      return NSS_STATUS_UNAVAIL;
+    }
+
   do
     {
       /* key is: "port/proto" */