From 7930ab49288141d4ebbe6bd58c8841f817891ed7 Mon Sep 17 00:00:00 2001 From: Asanka Herath Date: Fri, 16 Jul 2004 05:40:54 +0000 Subject: [PATCH] strsafe-20040715 String Safety fixes --- src/WINNT/afsd/afslogon.c | 49 +++++++++++++++++++++++-------------- src/WINNT/afsd/afslogon.h | 2 +- src/WINNT/afsd/logon_ad.cpp | 16 +++++------- 3 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/WINNT/afsd/afslogon.c b/src/WINNT/afsd/afslogon.c index 96bae52f8e..2a7ee9e998 100644 --- a/src/WINNT/afsd/afslogon.c +++ b/src/WINNT/afsd/afslogon.c @@ -56,7 +56,7 @@ void DebugEvent(char *a,char *b,...) a = AFS_DAEMON_EVENT_NAME; h = RegisterEventSource(NULL, a); va_start(marker,b); - _vsnprintf(buf,MAXBUF_,b,marker); + StringCbVPrintf(buf, MAXBUF_+1,b,marker); buf[MAXBUF_] = '\0'; ptbuf[0] = buf; ReportEvent(h, EVENTLOG_INFORMATION_TYPE, 0, 0, NULL, 1, 0, (const char **)ptbuf, NULL);\ @@ -331,7 +331,7 @@ void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, Log PSECURITY_LOGON_SESSION_DATA plsd; char lsaUsername[MAX_USERNAME_LENGTH]; char lsaDomain[MAX_DOMAIN_LENGTH]; - int len; + size_t len, tlen; LsaGetLogonSessionData(lpLogonId, &plsd); @@ -339,19 +339,28 @@ void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, Log UnicodeStringToANSI(plsd->LogonDomain, lsaDomain, MAX_DOMAIN_LENGTH); DebugEvent(NULL,"PLSD username[%s] domain[%s]",lsaUsername,lsaDomain); - DebugEvent(NULL,"PLSD Unicode username[%S] domain[%S]",plsd->UserName.Buffer,plsd->LogonDomain.Buffer); - DebugEvent(NULL,"PLSD lengths username[%d] domain[%d]",plsd->UserName.Length,plsd->LogonDomain.Length); - len = strlen(lsaUsername) + strlen(lsaDomain) + 2; + if(SUCCEEDED(StringCbLength(lsaUsername, MAX_USERNAME_LENGTH, &tlen))) + len = tlen; + else + goto bad_strings; + + if(SUCCEEDED(StringCbLength(lsaDomain, MAX_DOMAIN_LENGTH, &tlen))) + len += tlen; + else + goto bad_strings; + + len += 2; opt->smbName = malloc(len); - strcpy(opt->smbName, lsaDomain); - strcat(opt->smbName, "\\"); - strcat(opt->smbName, lsaUsername); + StringCbCopy(opt->smbName, len, lsaDomain); + StringCbCat(opt->smbName, len, "\\"); + StringCbCat(opt->smbName, len, lsaUsername); strlwr(opt->smbName); +bad_strings: LsaFreeReturnBuffer(plsd); } @@ -388,7 +397,10 @@ void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, Log WCHAR *wuname = NULL; HRESULT hr; - int len = strlen(opt->smbName) + 1; + size_t len; + + StringCbLength(opt->smbName, MAX_USERNAME_LENGTH, &len); + len ++; wuname = malloc(len * sizeof(WCHAR)); MultiByteToWideChar(CP_ACP,0,opt->smbName,-1,wuname,len*sizeof(WCHAR)); @@ -406,13 +418,15 @@ void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, Log DebugEvent(NULL,"Found logon script [%S]", regscript); if(dwType == REG_EXPAND_SZ) { + DWORD dwReq; + dwSize += MAX_PATH * sizeof(WCHAR); /* make room for environment expansion. */ regexscript = malloc(dwSize); - rv = ExpandEnvironmentStringsW(regscript, regexscript, dwSize / sizeof(WCHAR)); + dwReq = ExpandEnvironmentStringsW(regscript, regexscript, dwSize / sizeof(WCHAR)); free(regscript); regscript = regexscript; regexscript = NULL; - if(rv > (dwSize / sizeof(WCHAR))) { + if(dwReq > (dwSize / sizeof(WCHAR))) { DebugEvent(NULL,"Overflow while expanding environment strings."); goto doneLogonScript; } @@ -421,13 +435,12 @@ void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, Log DebugEvent(NULL,"After expanding env strings [%S]", regscript); if(wcsstr(regscript, L"%s")) { - dwSize += 256 * sizeof(WCHAR); /* make room for username expansion */ + dwSize += len * sizeof(WCHAR); /* make room for username expansion */ regexuscript = (WCHAR *) LocalAlloc(LMEM_FIXED, dwSize); hr = StringCbPrintfW(regexuscript, dwSize, regscript, wuname); } else { regexuscript = (WCHAR *) LocalAlloc(LMEM_FIXED, dwSize); - wcscpy(regexuscript, regscript); - hr = S_OK; + hr = StringCbCopyW(regexuscript, dwSize, regscript); } DebugEvent(NULL,"After expanding username [%S]", regexuscript); @@ -519,7 +532,6 @@ DWORD APIENTRY NPLogonNotify( MSV1_0_INTERACTIVE_LOGON *IL; DWORD code; - int len; int pw_exp; char *reason; @@ -616,7 +628,7 @@ DWORD APIENTRY NPLogonNotify( cell right away because the client service may not have started yet. This call also sets the AD_REALM flag in opt.flags if applicable. */ if(ISREMOTE(opt.flags)) - GetAdHomePath(homePath,MAX_PATH,lpLogonId,IL,&opt); + GetAdHomePath(homePath,MAX_PATH,lpLogonId,&opt); } /* loop until AFS is started. */ @@ -707,7 +719,8 @@ DWORD APIENTRY NPLogonNotify( if (code) { char msg[128]; - sprintf(msg, "Integrated login failed: %s", reason); + + StringCbPrintf(msg, sizeof(msg), "Integrated login failed: %s", reason); if (interactive && !opt.failSilently) MessageBox(hwndOwner, msg, "AFS Logon", MB_OK); @@ -761,7 +774,7 @@ VOID AFS_Logoff_Event( { DWORD code; if (code = ktc_ForgetAllTokens()) - DebugEvent("AFS AfsLogon - AFS_Logoff_Event - ForgetAllTokens failed [%lX]",code); + DebugEvent(NULL,"AFS AfsLogon - AFS_Logoff_Event - ForgetAllTokens failed [%lX]",code); else DebugEvent0("AFS AfsLogon - AFS_Logoff_Event - ForgetAllTokens succeeded"); } diff --git a/src/WINNT/afsd/afslogon.h b/src/WINNT/afsd/afslogon.h index b4a71397c6..108768b20e 100644 --- a/src/WINNT/afsd/afslogon.h +++ b/src/WINNT/afsd/afslogon.h @@ -118,7 +118,7 @@ static BOOL WINAPI UnicodeStringToANSI(UNICODE_STRING uInputString, LPSTR lpszOu void GetDomainLogonOptions( PLUID lpLogonId, char * username, char * domain, LogonOptions_t *opt ); DWORD GetFileCellName(char * path, char * cell, size_t cellLen); -DWORD GetAdHomePath(char * homePath, size_t homePathLen, PLUID lpLogonId, MSV1_0_INTERACTIVE_LOGON * IL, LogonOptions_t * opt); +DWORD GetAdHomePath(char * homePath, size_t homePathLen, PLUID lpLogonId, LogonOptions_t * opt); #ifdef __cplusplus } diff --git a/src/WINNT/afsd/logon_ad.cpp b/src/WINNT/afsd/logon_ad.cpp index 965e7e2379..7122be0c34 100644 --- a/src/WINNT/afsd/logon_ad.cpp +++ b/src/WINNT/afsd/logon_ad.cpp @@ -306,21 +306,16 @@ cleanup: /* Try to determine the user's AD home path. *homePath is assumed to be at least MAXPATH bytes. If successful, opt.flags is updated with LOGON_FLAG_AD_REALM to indicate that we are dealing with an AD realm. */ -DWORD GetAdHomePath(char * homePath, size_t homePathLen, PLUID lpLogonId, MSV1_0_INTERACTIVE_LOGON * IL, LogonOptions_t * opt) { +DWORD GetAdHomePath(char * homePath, size_t homePathLen, PLUID lpLogonId, LogonOptions_t * opt) { CtxtHandle ctx; + DWORD code = 0; SECURITY_STATUS status; + homePath[0] = '\0'; + if(LogonSSP(lpLogonId,&ctx)) return 1; else { - SecPkgContext_Names name; - status = QueryContextAttributes(&ctx,SECPKG_ATTR_NAMES,&name); - if(status != SEC_E_OK) { - DebugEvent(NULL,"Can't query names from context [%lX]",status); - goto ghp_0; - } - DebugEvent(NULL,"Context name [%s]",name.sUserName); - status = ImpersonateSecurityContext(&ctx); if(status == SEC_E_OK) { if(!QueryAdHomePath(homePath,homePathLen,lpLogonId)) { @@ -330,9 +325,10 @@ DWORD GetAdHomePath(char * homePath, size_t homePathLen, PLUID lpLogonId, MSV1_0 RevertSecurityContext(&ctx); } else { DebugEvent(NULL,"Can't impersonate context [%lX]",status); + code = 1; } ghp_0: DeleteSecurityContext(&ctx); - return 0; + return code; } }