Bug 40729 - xdm causes SEGVs setting up pam_response structure
xdm causes SEGVs setting up pam_response structure
Status: CLOSED ERRATA
Product: Red Hat Linux
Classification: Retired
Component: XFree86 (Show other bugs)
7.1
i386 Linux
medium Severity high
: ---
: ---
Assigned To: Mike A. Harris
David Lawrence
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-05-15 11:07 EDT by Ben Harris
Modified: 2007-04-18 12:33 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2001-11-01 08:28:13 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
This patch fixes the problem for me. (437 bytes, patch)
2001-05-15 11:08 EDT, Ben Harris
no flags Details | Diff

  None (edit)
Description Ben Harris 2001-05-15 11:07:05 EDT
Description of Problem:
Under some circumstances, xdm can be persuaded to catch a SIGSEGV.  I found
this while using some locally-developed modules (though it was pam_lastlog
that caught the SEGV) and I suspect that crafting a test case using the stock
modules will be tricky since it depends on the state of the malloc arena.

How Reproducible:
Easily if you have the right modules.  I can find the right set if it's really
necessary.

Additional Information:
The problem is in PAM_conv, in xc/programs/xdm/greeter/verify.c:

static int PAM_conv (int num_msg,
                     const struct pam_message **msg,
                     struct pam_response **resp,
                     void *appdata_ptr) {
        int replies = 0;
        struct pam_response *reply = NULL;

        reply = calloc(num_msg, sizeof(struct pam_response));
        if (!reply) return PAM_CONV_ERR;
#define COPY_STRING(s) (s) ? strdup(s) : NULL

        for (replies = 0; replies < num_msg; replies++) {
                switch (msg[replies]->msg_style) {
                case PAM_PROMPT_ECHO_OFF:
                        /* wants password */
                        reply[replies].resp_retcode = PAM_SUCCESS;
                        reply[replies].resp = COPY_STRING(PAM_password);
                        break;
                case PAM_TEXT_INFO:
                        /* ignore the informational mesage */
                        break;
                case PAM_PROMPT_ECHO_ON:
                        /* user name given to PAM already */
                        /* fall through */
                default:
                        /* unknown or PAM_ERROR_MSG */
                        free (reply);
                        return PAM_CONV_ERR;
                }
        }

#undef COPY_STRING
        *resp = reply;
        return PAM_SUCCESS;
}

There are two bugs here.  First, there are references to reply[replies], where
replies can be as large as num_msg-1, even though reply is only allocated
large enough to hold one pam_response.  This could cause heap corruption,
though I've not observed this.

The second problem is that in the case of a PAM_TEXT_INFO message, the resp
field of reply[replies] remains uninitialised, and thus when pam_lastlog
calls _pam_drop_reply() on the structure, it tries to free a random pointer,
with predictably nasty results.

A simple fix is to use calloc rather than malloc, and to pass it the number of
structures we need.  I'll attach a patch for this.
Comment 1 Ben Harris 2001-05-15 11:08:35 EDT
Created attachment 18450 [details]
This patch fixes the problem for me.
Comment 2 Mike A. Harris 2001-05-16 12:40:46 EDT
Nalin, can you look at this patch and comment on it.  Seems ok to me,
but I'd like security guru eyes to pass over it first.
Comment 3 Mike A. Harris 2001-11-01 08:28:07 EST
I have sent this patch upstream to XFree86 maintainers for application
to the CVS HEAD.  If accepted, it should appear in XFree86 4.2.0.
Comment 4 Mike A. Harris 2002-08-08 03:59:34 EDT
Did not appear in XFree86 4.2.0.  However that area of code did change,
and in an ugly (IMHO) way.

Another bug is that the return pointer from realloc() inside the GETMEM
macro in the new code, gets thrown away instead of being reassigned
to "reply", which assumes the realloc()'d memory will be in the same
location as the originally allocated block.

I've fixed both bugs with a new patch in rawhide 4.2.0-60.4 and later
Comment 5 Mark J. Cox (Product Security) 2003-06-25 11:52:40 EDT
An errata has been issued which should help the problem described in this bug report. 
This report is therefore being closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files, please follow the link below. You may reopen 
this bug report if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2003-066.html

Note You need to log in before you can comment on or make changes to this bug.