openssh bugs - Jan 2004 - [Bug 787] Minor security problem due to use of deprecated NGROUPS_MAX in uidswap.c (sshd)

http://bugzilla.mindrot.org/show_bug.cgi?id=787

           Summary: Minor security problem due to use of deprecated
                    NGROUPS_MAX in uidswap.c (sshd)
           Product: Portable OpenSSH
           Version: 3.7.1p2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: sshd
        AssignedTo: openssh-bugs at mindrot.org
        ReportedBy: holger at van-lengerich.de


I am able to produce the following behaviour by sshd on Solaris 2.8 and Linux
2.4.23, when NGROUPS_MAX at runtime is larger than at compile-time:

On both systems "fatal: getgroups: Invalid argument" gets logged via
syslog
and the sshd is terminating before any authentication is attempted. I located
the problem in uidswap.c, where deprecated NGROUPS_MAX is used.

NGOUPS_MAX is defined in limits.h and tells the maximum number  
of groups which an account can be member of. As NGROUPS_MAX is determined at
compile-time, this limit gets hardcoded into the resulting binary. As
NGROUPS_MAX may be larger at runtime than at compile-time it should be
regarded as deprecated and sysconf(_SC_NGROUPS_MAX) should be used instead.   
(see APUE 2.4.5 also)

In uidswap.c, line 41 NGROUPS_MAX is used to initialize static arrays in
global context. These 2 occurances of NGROUPS_MAX cannot be substituted
through sysconf(_SC_NGROUPS_MAX) easily as memory has to be allocated at
runtime.

In the same file NGROUPS_MAX is referenced in line 72 an 81. These occurances
can be replaced easily, once memory for the arrays is allocated according to  
sysconf(_SC_NGROUPS_MAX).

This bug also constitutes a minor security problem as it may be exploited to  
remotely enumerate accounts, which are member of more then NGROUPS_MAX at
compile-time.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=787

dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |793
              nThis|                            |
             Status|NEW                         |ASSIGNED



------- Additional Comments From dtucker at zip.com.au  2004-02-06 19:44 -------
I will look at this.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=787





------- Additional Comments From dtucker at zip.com.au  2004-02-06 20:44 -------
Created an attachment (id=539)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=539&action=view)
Use sysconf where available for NGROUPS_MAX.

Please try this patch.	Note: treat with *extreme* suspicion: it contains some
pointer-fu and compiled first try without warnings, so Murphy can't be far
away...



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=787





------- Additional Comments From djm at mindrot.org  2004-02-06 23:28 -------
(From update of attachment 539)
>Index: defines.h
>==================================================================>RCS
file: /usr/local/src/security/openssh/cvs/openssh_cvs/defines.h,v
>retrieving revision 1.109
>diff -u -p -r1.109 defines.h
>--- defines.h	27 Jan 2004 05:40:35 -0000	1.109
>+++ defines.h	6 Feb 2004 09:27:45 -0000
>@@ -541,6 +541,10 @@ struct winsize {
> # define SSH_SYSFDMAX 10000
> #endif
> 
>+#ifdef HAVE_SYSCONF
>+# undef NGROUPS_MAX
>+# define NGROUPS_MAX    (sysconf(_SC_NGROUPS_MAX))
I think that should be:

#if defined(HAVE_SYSCONF) && defined(_SC_NGROUPS_MAX)

We also need to check for sysconf returning -1

I'm wary of this change for 3.8. 

Perhaps a static check for gid >= NGROUPS_MAX?




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.