nis: nis_local_group may read from __nisgroup[-1] (bug 28075)

Message ID 87tul0ex4t.fsf@oldenburg.str.redhat.com
State Committed
Commit 70099c7763b8c47a4bb1eef1c41601c602a83270
Headers
Series nis: nis_local_group may read from __nisgroup[-1] (bug 28075) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Florian Weimer July 11, 2021, 2:35 p.m. UTC
  Tested on i686-linux-gnu and x86_64-linux-gnu.

---
 nis/nis_local_names.c | 1 +
 1 file changed, 1 insertion(+)
  

Comments

Carlos O'Donell July 11, 2021, 2:58 p.m. UTC | #1
On 7/11/21 10:35 AM, Florian Weimer via Libc-alpha wrote:
> Tested on i686-linux-gnu and x86_64-linux-gnu.

In nis/* there are 6 uses of getenv.

The other users: __nis_default_owner, __nis_default_group,
__nis_default_ttl, __nis_default_access, all use strstr to
determine the position of parameters and don't suffer the
same problem (empty env var), likewise nis_getnames but with
__srtok_r. The code you are fixing doesn't appear to consider
an empty env var. I also looked up the call chain to see if
nis_clone_object inspected the group to see if it was empty
and it does, and so it appears safe.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
 
> ---
>  nis/nis_local_names.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/nis/nis_local_names.c b/nis/nis_local_names.c
> index 080dcf88bd..218764f32e 100644
> --- a/nis/nis_local_names.c
> +++ b/nis/nis_local_names.c
> @@ -31,6 +31,7 @@ nis_local_group (void)
>    char *cptr;
>    if (__nisgroup[0] == '\0'
>        && (cptr = getenv ("NIS_GROUP")) != NULL
> +      && cptr[0] != '\0'
>        && strlen (cptr) < NIS_MAXNAMELEN)
>      {
>        char *cp = stpcpy (__nisgroup, cptr);
>
  

Patch

diff --git a/nis/nis_local_names.c b/nis/nis_local_names.c
index 080dcf88bd..218764f32e 100644
--- a/nis/nis_local_names.c
+++ b/nis/nis_local_names.c
@@ -31,6 +31,7 @@  nis_local_group (void)
   char *cptr;
   if (__nisgroup[0] == '\0'
       && (cptr = getenv ("NIS_GROUP")) != NULL
+      && cptr[0] != '\0'
       && strlen (cptr) < NIS_MAXNAMELEN)
     {
       char *cp = stpcpy (__nisgroup, cptr);