Bug 9807 - wbinfo --pam-logon segfaults on failure
Summary: wbinfo --pam-logon segfaults on failure
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.6
Classification: Unclassified
Component: Winbind (show other bugs)
Version: 3.6.13
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-17 15:49 UTC by David Disseldorp
Modified: 2013-04-22 09:35 UTC (History)
0 users

See Also:


Attachments
patch for 3.6 and 4.0 branches (1.63 KB, patch)
2013-04-17 20:28 UTC, David Disseldorp
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2013-04-17 15:49:04 UTC
wbinfo_pam_logon() calls wbcLogonUser(&error) with and expects the error pointer to be allocated on failure.
nsswitch/wbinfo.c:
1748 static bool wbinfo_pam_logon(char *username)
1749 {
...
1752         struct wbcAuthErrorInfo *error;
...
1796         wbc_status = wbcLogonUser(&params, NULL, &error, NULL);
...
1803         if (!WBC_ERROR_IS_OK(wbc_status)) {
1804                 d_fprintf(stderr,
1805                           "error code was %s (0x%x)\nerror message was: %s\n",
1806                           error->nt_string,
1807                           (int)error->nt_status,
1808                           error->display_string);


It's incorrect to assume error is allocated, wbcLogonUser() only does so if the nt_status response is non-zero.
nsswitch/libwbclient/wbc_pam.c:
1037 wbcErr wbcLogonUser(const struct wbcLogonUserParams *params,
1038                     struct wbcLogonUserInfo **info,
1039                     struct wbcAuthErrorInfo **error,
1040                     struct wbcUserPasswordPolicyInfo **policy)
1041 {       
...
142         wbc_status = wbcRequestResponse(WINBINDD_PAM_AUTH,
1143                                         &request,
1144                                         &response);
1145 
1146         if (response.data.auth.nt_status != 0) {
1147                 if (error) {
1148                         wbc_status = wbc_create_error_info(&response,
1149                                                            error);
1150                         BAIL_ON_WBC_ERROR(wbc_status);
1151                 }
1152 
1153                 wbc_status = WBC_ERR_AUTH_ERROR;
1154                 BAIL_ON_WBC_ERROR(wbc_status);
1155         }
1156         BAIL_ON_WBC_ERROR(wbc_status);
...
1170 done:
1171         winbindd_free_response(&response);
1172 
1173         return wbc_status;

Patch to follow.
Comment 1 David Disseldorp 2013-04-17 20:28:31 UTC
Created attachment 8783 [details]
patch for 3.6 and 4.0 branches

Same change as is in master.
Comment 2 Jeremy Allison 2013-04-17 20:57:48 UTC
Comment on attachment 8783 [details]
patch for 3.6 and 4.0 branches

LGTM for 3.6.next and 4.0.next
Comment 3 Jeremy Allison 2013-04-17 20:58:22 UTC
Re-assigning to Karolin for inclusion in 3.6.next and 4.0.next.
Jeremy.
Comment 4 Karolin Seeger 2013-04-22 07:42:41 UTC
Pushed to v3-6-test and autobuild-v4-0-test.
Comment 5 Karolin Seeger 2013-04-22 09:35:04 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!