Author: branden
Date: 2003-09-25 00:53:54 -0500 (Thu, 25 Sep 2003)
New Revision: 583

Modified:
   trunk/debian/changelog
   trunk/debian/patches/000_stolen_from_HEAD.diff
   trunk/debian/patches/002_xdm_fixes.diff
Log:
SECURITY: Fix for CAN-2003-0690; xdm did not verify the return value of
the pam_setcred() function, which could allow attackers to gain root
privileges by triggering error conditions within PAM modules, as
demonstrated in certain configurations of the MIT pam_krb5 module.
Fix backported from XFree86 CVS xf-4_3-branch: xdm now checks the return
value of pam_setcred(), issues an error message if it is not zero, and
treats this situation as an authentication failure.

- debian/patches/000_stolen_from_HEAD.diff: added patch
- debian/patches/002_xdm_fixes.diff: resynced offsets; update
  pam_setcred() patch to use pam_error and pam_strerror() when reporting
  problems


Modified: trunk/debian/changelog
===================================================================
--- trunk/debian/changelog      2003-09-25 04:16:01 UTC (rev 582)
+++ trunk/debian/changelog      2003-09-25 05:53:54 UTC (rev 583)
@@ -1,7 +1,7 @@
 xfree86 (4.2.1-12) unstable; urgency=high
 
   * urgency high due to security fix for font service integer overflow attacks
-    (see below)
+    and xdm's failure to verify return value of pam_setcred() (see below)
 
   * debian/twm.menu-method: correctly escape doublequote characters in menu
     entry titles (thanks, Bill Allombert) (Closes: #193759)
@@ -242,8 +242,20 @@
     UTF-8.
     - debian/{copyright,changelog,changelog.Debian.old}: recoded with iconv
 
- -- Branden Robinson <[EMAIL PROTECTED]>  Wed, 24 Sep 2003 15:56:28 -0500
+  * SECURITY: Fix for CAN-2003-0690; xdm did not verify the return value of
+    the pam_setcred() function, which could allow attackers to gain root
+    privileges by triggering error conditions within PAM modules, as
+    demonstrated in certain configurations of the MIT pam_krb5 module.
+    Fix backported from XFree86 CVS xf-4_3-branch: xdm now checks the return
+    value of pam_setcred(), issues an error message if it is not zero, and
+    treats this situation as an authentication failure.
+    - debian/patches/000_stolen_from_HEAD.diff: added patch
+    - debian/patches/002_xdm_fixes.diff: resynced offsets; update
+      pam_setcred() patch to use pam_error and pam_strerror() when reporting
+      problems
 
+ -- Branden Robinson <[EMAIL PROTECTED]>  Wed, 24 Sep 2003 23:07:26 -0500
+
 xfree86 (4.2.1-11) unstable; urgency=medium
 
   * urgency set to medium because bug #206790 bites a lot of people (but,

Modified: trunk/debian/patches/000_stolen_from_HEAD.diff
===================================================================
--- trunk/debian/patches/000_stolen_from_HEAD.diff      2003-09-25 04:16:01 UTC (rev 
582)
+++ trunk/debian/patches/000_stolen_from_HEAD.diff      2003-09-25 05:53:54 UTC (rev 
583)
@@ -5411,3 +5411,18 @@
        }
  
      if (versions[version_index].major_version > Dont_Check)
+--- xc/programs/xdm/session.c~ 2003-09-24 17:23:56.000000000 -0500
++++ xc/programs/xdm/session.c  2003-09-24 17:24:02.000000000 -0500
+@@ -599,7 +599,11 @@
+ #endif   /* QNX4 doesn't support multi-groups, no initgroups() */
+ #ifdef USE_PAM
+       if (thepamh()) {
+-          pam_setcred(thepamh(), PAM_ESTABLISH_CRED);
++          if (pam_setcred(thepamh(), PAM_ESTABLISH_CRED) != PAM_SUCCESS) {
++              LogError("pam_setcred for %\"s failed, errno=%d\n",
++                       name, errno);
++              return(0);
++          }
+       }
+ #endif
+       if (setuid(verify->uid) < 0)

Modified: trunk/debian/patches/002_xdm_fixes.diff
===================================================================
--- trunk/debian/patches/002_xdm_fixes.diff     2003-09-25 04:16:01 UTC (rev 582)
+++ trunk/debian/patches/002_xdm_fixes.diff     2003-09-25 05:53:54 UTC (rev 583)
@@ -95,7 +95,8 @@
     Use _SysErrorMsg() instead of strerror().
 * session.c:
     Indent pre-processor statements.  Use _SysErrorMsg() instead of
-    strerror().  Recognize that GNU LibC-based systems as well as Linux
+    strerror(), except when handling PAM errors, when we use
+    pam_strerror().  Recognize that GNU LibC-based systems as well as Linux
     systems in general have the endpwent() function.  Recognize that GNU
     LibC-based systems as well as CSRG_BASED ones have the getpwnam() and
     crypt() functions.  Style fix: put function names flush left when
@@ -1394,9 +1395,8 @@
        } else {
            user_pass = sp->sp_pwdp;
        }
-diff -urN xc/programs/xdm~/session.c xc/programs/xdm/session.c
---- xc/programs/xdm~/session.c 2001-12-14 15:01:23.000000000 -0500
-+++ xc/programs/xdm/session.c  2003-02-23 17:04:25.000000000 -0500
+--- xc/programs/xdm/session.c~ 2003-09-25 00:40:10.000000000 -0500
++++ xc/programs/xdm/session.c  2003-09-25 00:43:33.000000000 -0500
 @@ -60,17 +60,17 @@
  #endif
  
@@ -1490,8 +1490,70 @@
  #endif
  
  static void
-@@ -577,35 +583,33 @@
+@@ -246,7 +252,7 @@
+ static int
+ IOErrorHandler (Display *dpy)
+ {
+-    LogError("fatal IO error %d (%s)\n", errno, _SysErrorMsg(errno));
++    LogError ("fatal IO error %d (%s)\n", errno, _SysErrorMsg(errno));
+     exit(RESERVER_DISPLAY);
+     /*NOTREACHED*/
+     return 0;
+@@ -255,7 +261,7 @@
+ static int
+ ErrorHandler(Display *dpy, XErrorEvent *event)
+ {
+-    LogError("X error\n");
++    LogError ("X error\n");
+     if (XmuPrintDefaultErrorMessage (dpy, event, stderr) == 0) return 0;
+     exit(UNMANAGE_DISPLAY);
+     /*NOTREACHED*/
+@@ -288,13 +294,13 @@
+ #ifdef GREET_USER_STATIC
+     greet_user_proc = GreetUser;
+ #else
+-    Debug("ManageSession: loading greeter library %s\n", greeterLib);
++    Debug ("ManageSession: loading greeter library %s\n", greeterLib);
+     greet_lib_handle = dlopen(greeterLib, RTLD_NOW);
+     if (greet_lib_handle != NULL)
+       greet_user_proc = (GreetUserProc)dlsym(greet_lib_handle, "GreetUser");
+     if (greet_user_proc == NULL)
+       {
+-      LogError("%s while loading %s\n", dlerror(), greeterLib);
++      LogError ("%s while loading %s\n", dlerror(), greeterLib);
+       exit(UNMANAGE_DISPLAY);
+       }
+ #endif
+@@ -500,7 +506,7 @@
  
+           code = Krb5DisplayCCache(d->name, &ccache);
+           if (code)
+-              LogError("%s while getting Krb5 ccache to destroy\n",
++              LogError ("%s while getting Krb5 ccache to destroy\n",
+                        error_message(code));
+           else {
+               code = krb5_cc_destroy(ccache);
+@@ -508,8 +514,8 @@
+                   if (code == KRB5_FCC_NOFILE) {
+                       Debug ("No Kerberos ccache file found to destroy\n");
+                   } else
+-                      LogError("%s while destroying Krb5 credentials cache\n",
+-                               error_message(code));
++                      LogError ("%s while destroying Krb5 credentials"
++                                " cache\n", error_message(code));
+               } else
+                   Debug ("Kerberos ccache destroyed\n");
+               krb5_cc_close(ccache);
+@@ -537,6 +543,7 @@
+ #endif
+ #ifdef USE_PAM 
+     pam_handle_t *pamh = thepamh();
++    int pam_error;
+ #endif
+ 
+     if (verify->argv) {
+@@ -577,39 +584,38 @@
+ 
  #ifndef AIXV3
  #ifndef HAS_SETUSERCONTEXT
 -      if (setgid(verify->gid) < 0)
@@ -1526,9 +1588,17 @@
  #endif   /* QNX4 doesn't support multi-groups, no initgroups() */
  #ifdef USE_PAM
 -      if (thepamh()) {
--          pam_setcred(thepamh(), PAM_ESTABLISH_CRED);
+-          if (pam_setcred(thepamh(), PAM_ESTABLISH_CRED) != PAM_SUCCESS) {
+-              LogError("pam_setcred for %\"s failed, errno=%d\n",
+-                       name, errno);
+-              return(0);
 +      if (thepamh ()) {
-+          pam_setcred (thepamh (), PAM_ESTABLISH_CRED);
++          pam_error = pam_setcred (thepamh (), PAM_ESTABLISH_CRED);
++          if (pam_error != PAM_SUCCESS) {
++              LogError ("pam_setcred for \"%s\" failed: %s\n", name,
++                        pam_strerror (pam_error));
++              return (0);
+           }
        }
  #endif
 -      if (setuid(verify->uid) < 0)
@@ -1541,7 +1611,7 @@
            return (0);
        }
  #else /* HAS_SETUSERCONTEXT */
-@@ -613,20 +617,17 @@
+@@ -617,20 +623,17 @@
         * Set the user's credentials: uid, gid, groups,
         * environment variables, resource limits, and umask.
         */
@@ -1571,7 +1641,7 @@
            return (0);
        }
  #endif /* HAS_SETUSERCONTEXT */
-@@ -635,9 +636,9 @@
+@@ -639,9 +642,9 @@
         * Set the user's credentials: uid, gid, groups,
         * audit classes, user limits, and umask.
         */
@@ -1584,7 +1654,7 @@
            return (0);
        }
  #endif /* AIXV3 */
-@@ -751,13 +752,13 @@
+@@ -755,13 +758,13 @@
        execute (failsafeArgv, verify->userEnviron);
        exit (1);
      case -1:
@@ -1602,7 +1672,7 @@
        Debug ("StartSession, fork succeeded %d\n", pid);
        *pidp = pid;
        return 1;
-@@ -924,9 +925,10 @@
+@@ -928,9 +931,10 @@
      return env;
  }
  
@@ -1616,7 +1686,6 @@
 +    return (s2);
  }
  #endif
-diff -urN xc/programs/xdm~/socket.c xc/programs/xdm/socket.c
 --- xc/programs/xdm~/socket.c  2001-12-14 15:01:24.000000000 -0500
 +++ xc/programs/xdm/socket.c   2003-02-23 17:04:25.000000000 -0500
 @@ -71,7 +71,7 @@


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to