From: Jeffrey Altman Date: Tue, 20 Jan 2009 04:35:53 +0000 (+0000) Subject: windows-smb-parse-ascii-block-20090119 X-Git-Tag: openafs-devel-1_5_61~599 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=a0fd9b17334333e257ae9c476c4fbf85ee9fec95 windows-smb-parse-ascii-block-20090119 LICENSE MIT smb_ParseASCIIBlock() may be called with more than just data type ASCII (0x4). If the type is 2, 3, or 4 the data field is a null terminated string. If the type is 1 or 5 the data field is a counted string. if the type is 2 the data field is never Unicode. In any case, if the type is unrecognized smb_ParseASCIIBlock() will return NULL and all of the callers must be prepared to handle it. If the return is NULL, the smb request will fail with CM_ERROR_BADSMB. --- diff --git a/src/WINNT/afsd/smb.c b/src/WINNT/afsd/smb.c index 50dc9b4..7df15ac 100644 --- a/src/WINNT/afsd/smb.c +++ b/src/WINNT/afsd/smb.c @@ -2613,22 +2613,42 @@ clientchar_t *smb_ParseASCIIBlock(smb_packet_t * pktp, unsigned char *inp, char **chainpp, int flags) { size_t cb; + afs_uint32 type = *inp++; - if (*inp++ != 0x4) - return NULL; + /* + * The first byte specifies the type of the input string. + * CIFS TR 1.0 3.2.10. This function only parses null terminated + * strings. + */ + switch (type) { + /* Length Counted */ + case 0x1: /* Data Block */ + case 0x5: /* Variable Block */ + cb = *inp++ << 16 | *inp++; + break; + + /* Null-terminated string */ + case 0x4: /* ASCII */ + case 0x3: /* Pathname */ + case 0x2: /* Dialect */ + cb = sizeof(pktp->data) - (inp - pktp->data); + if (inp < pktp->data || inp >= pktp->data + sizeof(pktp->data)) { +#ifdef DEBUG_UNICODE + DebugBreak(); +#endif + cb = sizeof(pktp->data); + } + break; + + default: + return NULL; /* invalid input */ + } #ifdef SMB_UNICODE - if (!WANTS_UNICODE(pktp)) + if (type == 0x2 /* Dialect */ || !WANTS_UNICODE(pktp)) flags |= SMB_STRF_FORCEASCII; #endif - cb = sizeof(pktp->data) - (inp - pktp->data); - if (inp < pktp->data || inp >= pktp->data + sizeof(pktp->data)) { -#ifdef DEBUG_UNICODE - DebugBreak(); -#endif - cb = sizeof(pktp->data); - } return smb_ParseStringBuf(pktp->data, &pktp->stringsp, inp, &cb, chainpp, flags); } @@ -4076,6 +4096,8 @@ long smb_ReceiveCoreTreeConnect(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * char *tbp; tbp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tbp, &tbp, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; } tp = cm_ClientStrRChr(pathp, '\\'); if (!tp) @@ -4247,7 +4269,8 @@ long smb_ReceiveCoreSearchVolume(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH|SMB_STRF_FORCEASCII); - osi_assertx(pathp != NULL, "null path"); + if (!pathp) + return CM_ERROR_BADSMB; statBlockp = smb_ParseVblBlock(tp, &tp, &statLen); osi_assertx(statBlockp != NULL, "null statBlock"); if (statLen == 0) { @@ -4544,12 +4567,12 @@ long smb_ReceiveCoreSearchDir(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *ou tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH|SMB_STRF_FORCEASCII); - inCookiep = smb_ParseVblBlock(tp, &tp, &dataLength); + if (!pathp) + return CM_ERROR_BADSMB; - /* bail out if request looks bad */ - if (!tp || !pathp) { + inCookiep = smb_ParseVblBlock(tp, &tp, &dataLength); + if (!tp) return CM_ERROR_BADSMB; - } /* We can handle long names */ if (vcp->flags & SMB_VCFLAG_USENT) @@ -5057,7 +5080,7 @@ long smb_ReceiveCoreCheckPath(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *ou pdata = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, pdata, NULL, SMB_STRF_ANSIPATH); if (!pathp) - return CM_ERROR_BADFD; + return CM_ERROR_BADSMB; osi_Log1(smb_logp, "SMB receive check path %S", osi_LogSaveClientString(smb_logp, pathp)); @@ -5428,6 +5451,8 @@ long smb_ReceiveCoreOpen(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) datap = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, datap, NULL, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; osi_Log1(smb_logp, "SMB receive open file [%S]", osi_LogSaveClientString(smb_logp, pathp)); @@ -5655,6 +5680,8 @@ long smb_ReceiveCoreUnlink(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; osi_Log1(smb_logp, "SMB receive unlink %S", osi_LogSaveClientString(smb_logp, pathp)); @@ -6207,7 +6234,11 @@ smb_ReceiveCoreRename(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) tp = smb_GetSMBData(inp, NULL); oldPathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); + if (!oldPathp) + return CM_ERROR_BADSMB; newPathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); + if (!newPathp) + return CM_ERROR_BADSMB; osi_Log2(smb_logp, "smb rename [%S] to [%S]", osi_LogSaveClientString(smb_logp, oldPathp), @@ -6301,6 +6332,8 @@ long smb_ReceiveCoreRemoveDir(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *ou tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; spacep = inp->spacep; smb_StripLastComponent(spacep->wdata, &lastNamep, pathp); @@ -7691,13 +7724,15 @@ long smb_ReceiveCoreMakeDir(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); - - if (cm_ClientStrCmp(pathp, _C("\\")) == 0) - return CM_ERROR_EXISTS; + if (!pathp) + return CM_ERROR_BADSMB; spacep = inp->spacep; smb_StripLastComponent(spacep->wdata, &lastNamep, pathp); + if (cm_ClientStrCmp(pathp, _C("\\")) == 0) + return CM_ERROR_EXISTS; + userp = smb_GetUserFromVCP(vcp, inp); caseFold = CM_FLAG_CASEFOLD; @@ -7820,6 +7855,8 @@ long smb_ReceiveCoreCreate(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) tp = smb_GetSMBData(inp, NULL); pathp = smb_ParseASCIIBlock(inp, tp, &tp, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; if (!cm_IsValidClientString(pathp)) { #ifdef DEBUG diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index 9976d10..4463f6e 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -5673,6 +5673,8 @@ long smb_ReceiveV3OpenX(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) pathp = smb_ParseASCIIBlock(inp, smb_GetSMBData(inp, NULL), NULL, SMB_STRF_ANSIPATH); + if (!pathp) + return CM_ERROR_BADSMB; spacep = inp->spacep; smb_StripLastComponent(spacep->wdata, &lastNamep, pathp); @@ -8890,7 +8892,11 @@ long smb_ReceiveNTRename(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t *outp) tp = smb_GetSMBData(inp, NULL); oldPathp = smb_ParseASCIIBlock(inp, tp, &tp, 0); + if (!oldPathp) + return CM_ERROR_BADSMB; newPathp = smb_ParseASCIIBlock(inp, tp, &tp, 0); + if (!newPathp) + return CM_ERROR_BADSMB; osi_Log3(smb_logp, "NTRename for [%S]->[%S] type [%s]", osi_LogSaveClientString(smb_logp, oldPathp),