View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000447 | Far Manager | Editor | public | 2007-12-18 09:16 | 2008-01-10 00:12 |
Reporter | alexmitin | Assigned To | vskirdin | ||
Priority | normal | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Fixed in Version | 1.80 alpha 1 | ||||
Summary | 0000447: Buffer overrun in Edit:ProcessKey() | ||||
Description | Steps to reproduce ====================== 1) Start FAR under debugger. 2) Put a breakpoint on the following line: wchar_t *ShortStr=new wchar_t[StrSize+1]; See the code below for the full context. 3) Press Ctrl+A on the file or folder 4) Select "File modification date" control. 5) Press Right arrow button. 6) Step through the code and watch the value of "ShortStr" variable. File: edit.cpp Line: 1024-1025 Context: int Edit::ProcessKey(int Key) { ... case KEY_RIGHT: case KEY_NUMPAD6: case KEY_CTRLD: { PrevCurPos=CurPos; if (Mask && *Mask) { wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; wcsncpy(ShortStr,Str,StrSize); <==[bug]== int Len=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; if (Len>CurPos) CurPos++; } ... } Analysis ========= The strncpy function copies the initial count characters of strSource to strDest and returns strDest. If count is less than or equal to the length of strSource, a null character is not appended automatically to the copied string. If count is greater than the length of strSource, the destination string is padded with null characters up to length count. The behavior of strncpy is undefined if the source and destination strings overlap. See http://msdn2.microsoft.com/en-us/library/xdsywd25(VS.80).aspx ----- In my test run the value of the variables were the following: StrSize=10 Str=12/18/2007 In this particular case the function "wcsncpy" does not null terminate "ShortStr". The next line calls RemoveTrailingSpaces, which in turns calls StrLength, which calls lstrlenW. Because "ShortStr" is not null terminated lstrlenW reads after the allocated buffer. In some cases it can cause access violation error. There are several other places like this in the Edit::ProcessKey function. They all should be fixed. Fix =========== wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; lstrcpynW(ShortStr,Str,StrSize+1); <==[fix]== int Len=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; | ||||
Additional Information | FAR 1.80.0.394 | ||||
Tags | No tags attached. | ||||
Build | 407 | ||||
|
есть же в strncpy.c xwcsncpy() |
|
после 395 |
|
Валь, не то. См. аттач. |
2007-12-24 01:25
|
work.diff (8,437 bytes)
Index: panel.cpp =================================================================== --- panel.cpp (revision 1406) +++ panel.cpp (working copy) @@ -969,8 +969,7 @@ { wchar_t RootDir[10]; //BUGBUG - wcsncpy(RootDir,strCurDir,3); - RootDir[3]=0; + xwcsncpy(RootDir,strCurDir,3); if ( FAR_GetDriveType(RootDir) == DRIVE_NO_ROOT_DIR ) ChDisk.ClearDone(); Index: edit.cpp =================================================================== --- edit.cpp (revision 1406) +++ edit.cpp (working copy) @@ -765,7 +765,7 @@ if (ShortStr==NULL) return FALSE; - wcsncpy(ShortStr,Str,StrSize); + xwcsncpy(ShortStr,Str,StrSize); Len=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; } @@ -988,7 +988,7 @@ wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; - wcsncpy(ShortStr,Str,StrSize); + xwcsncpy(ShortStr,Str,StrSize); CurPos=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; } @@ -1021,7 +1021,7 @@ wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; - lstrcpynW(ShortStr,Str,StrSize+1); + xwcsncpy(ShortStr,Str,StrSize); int Len=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; if (Len>CurPos) @@ -1117,7 +1117,7 @@ wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; - wcsncpy(ShortStr,Str,StrSize); + xwcsncpy(ShortStr,Str,StrSize); Len=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; if (Len>CurPos) @@ -1162,7 +1162,7 @@ wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return FALSE; - wcsncpy(ShortStr,Str,StrSize); + xwcsncpy(ShortStr,Str,StrSize); RemoveTrailingSpaces(ShortStr); CopyToClipboard(ShortStr); delete[] ShortStr; @@ -1443,7 +1443,6 @@ void Edit::GetString(wchar_t *Str,int MaxSize) { xwcsncpy(Str, Edit::Str,MaxSize-1); - Str[MaxSize-1]=0; } void Edit::GetString(string &strStr) @@ -1612,8 +1611,7 @@ else CopyLength=Min(MaxSize-1,SelEnd-SelStart); - wcsncpy(Str,Edit::Str+SelStart,CopyLength); - Str[CopyLength]=0; + xwcsncpy(Str,Edit::Str+SelStart,CopyLength); return(TRUE); } @@ -1633,8 +1631,7 @@ wchar_t *lpwszStr = strStr.GetBuffer (CopyLength+1); - wcsncpy(lpwszStr,Edit::Str+SelStart,CopyLength); - lpwszStr[CopyLength]=0; + xwcsncpy(lpwszStr,Edit::Str+SelStart,CopyLength); strStr.ReleaseBuffer (); @@ -1976,7 +1973,7 @@ wchar_t *ShortStr=new wchar_t[StrSize+1]; if (ShortStr==NULL) return; - wcsncpy(ShortStr,Str,StrSize); + xwcsncpy(ShortStr,Str,StrSize); Pos=StrLength(RemoveTrailingSpaces(ShortStr)); delete[] ShortStr; if (NewPos>Pos) Index: mix.cpp =================================================================== --- mix.cpp (revision 1406) +++ mix.cpp (working copy) @@ -226,7 +226,7 @@ if ( Result && IsAlpha (*lpwszBuffer) && lpwszBuffer[1] == L':' && - (lpwszBuffer[2] == 0 || lpwszBuffer[2] == '\\') + (lpwszBuffer[2] == 0 || lpwszBuffer[2] == L'\\') ) *lpwszBuffer = Upper (*lpwszBuffer); @@ -443,7 +443,7 @@ int nResult = ConvertWildcards(param1, strResult, flags & 0xFFFF); - xwcsncpy(param2, strResult, size); + xwcsncpy(param2, strResult, size); //?? � ����� �� size-1 return nResult; } @@ -1066,7 +1066,7 @@ if ( FarMkTempEx(strDest, Prefix, TRUE) ) { - xwcsncpy (Dest, strDest, size); + xwcsncpy (Dest, strDest, size); //?? � ����� �� size-1 return Dest; } @@ -1121,7 +1121,7 @@ if(DriveType == DRIVE_UNKNOWN) { - LocalName[2]='\\'; + LocalName[2]=L'\\'; DriveType = FAR_GetDriveType(LocalName); LocalName[2]=0; } @@ -1768,8 +1768,7 @@ */ wchar_t LocalName[3]; - wcsncpy (LocalName, CurDir, 2); - LocalName[2] = 0; + xwcsncpy (LocalName, CurDir, 2); apiWNetGetConnection (LocalName, strNetDir); } Index: fnparce.cpp =================================================================== --- fnparce.cpp (revision 1406) +++ fnparce.cpp (working copy) @@ -836,7 +836,7 @@ if(Modifers && *Modifers) { - if(wcschr(Modifers,'F') && PointToName((const wchar_t*)strFileName) == (const wchar_t*)strFileName) // 'F' - ���������� ����� ���; //BUGBUG + if(wcschr(Modifers,L'F') && PointToName((const wchar_t*)strFileName) == (const wchar_t*)strFileName) // 'F' - ���������� ����� ���; //BUGBUG { string strTempFileName; strTempFileName.Format (L"%s%s%s", (const wchar_t*)strCurDir,(strCurDir.At(StrLength(strCurDir)-1) != L'\\'?L"\\":L""), (const wchar_t*)strFileName); //BUGBUG Index: syntax.cpp =================================================================== --- syntax.cpp (revision 1406) +++ syntax.cpp (working copy) @@ -300,7 +300,7 @@ static wchar_t buffer[128]; wchar_t *p = buffer; int ch; - while ( ( ( ch = getChar() ) != EOFCH ) && (iswxdigit(ch) || ch == 'x') && ( (p-buffer) < 32 )) + while ( ( ( ch = getChar() ) != EOFCH ) && (iswxdigit(ch) || ch == L'x') && ( (p-buffer) < 32 )) *p++ = (char)ch; *p = 0; putBack(ch); @@ -725,8 +725,7 @@ wchar_t *CurKeyText = strCurKeyText.GetBuffer (Length+1); - wcsncpy(CurKeyText,CurBufPtr,Length); - CurKeyText[Length] = 0; + xwcsncpy(CurKeyText,CurBufPtr,Length); strCurKeyText.ReleaseBuffer (); Index: macro.cpp =================================================================== --- macro.cpp (revision 1406) +++ macro.cpp (working copy) @@ -1276,7 +1276,7 @@ p = es; if ( (nFlags & FLAG_DISK) == FLAG_DISK ) - wcsncpy (lpDest, s, p-s); + wcsncpy (lpDest, s, p-s); //??! xwcsncpy() } else { Index: UNICODE =================================================================== --- UNICODE (revision 1406) +++ UNICODE (working copy) @@ -14,7 +14,7 @@ strcat wcscat strncat wcsncat strcpy wcscpy - strncpy wcsncpy + xstrncpy xwcsncpy see file "strncpy.c" strcmp StrCmp _stricmp StrCmpI strncmp StrCmpN Index: clipboard.cpp =================================================================== --- clipboard.cpp (revision 1406) +++ clipboard.cpp (working copy) @@ -341,7 +341,6 @@ wmemset(ClipText,0,BufferSize+2); xwcsncpy(ClipText,ClipAddr,BufferSize); - ClipText[BufferSize]=0; /* if (Unicode) if(AnsiMode) Index: strmix.cpp =================================================================== --- strmix.cpp (revision 1406) +++ strmix.cpp (working copy) @@ -257,8 +257,7 @@ PartBeforeName = (wchar_t*)xf_malloc ((BeforeNameLength+1)*sizeof (wchar_t)); - wcsncpy(PartBeforeName, Src, BeforeNameLength); - PartBeforeName[BeforeNameLength]=0; + xwcsncpy(PartBeforeName, Src, BeforeNameLength); const wchar_t *SrcNameDot=wcsrchr(SrcNamePtr, L'.'); @@ -288,7 +287,7 @@ *(DestNamePtr++)=*(SrcNamePtr++); } break; - case '.': + case L'.': CurWildPtr++; *(DestNamePtr++)=L'.'; if (wcspbrk(CurWildPtr,L"*?")!=NULL) Index: editor.cpp =================================================================== --- editor.cpp (revision 1406) +++ editor.cpp (working copy) @@ -3538,7 +3538,6 @@ const wchar_t *Str=CurPtr->GetStringAddrW()+CurPtr->GetCurPos(); wchar_t *TmpStr=new wchar_t[SearchLength+1]; xwcsncpy(TmpStr,Str,SearchLength); - TmpStr[SearchLength]=0; /* if (UseDecodeTable) @@ -4279,7 +4278,6 @@ UndoData[UndoDataPos].StrPos=StrPos; UndoData[UndoDataPos].StrNum=StrNum; xwcsncpy(UndoData[UndoDataPos].EOL,Eol?Eol:L"",countof(UndoData[UndoDataPos].EOL)-1); - UndoData[UndoDataPos].EOL[countof(UndoData[UndoDataPos].EOL)-1]=0; if (Str!=NULL) { |
|
Так что с аттачем будем делать? |
|
после 407 |
Date Modified | Username | Field | Change |
---|---|---|---|
2007-12-18 09:16 | alexmitin | New Issue | |
2007-12-18 09:16 | alexmitin | Status | new => assigned |
2007-12-18 09:16 | alexmitin | Assigned To | => vskirdin |
2007-12-18 09:55 | samlyukov | Note Added: 0001051 | |
2007-12-21 05:41 | vskirdin | Note Added: 0001055 | |
2007-12-21 05:41 | vskirdin | Status | assigned => feedback |
2007-12-24 01:24 | samlyukov | Note Added: 0001071 | |
2007-12-24 01:25 | samlyukov | File Added: work.diff | |
2008-01-09 10:18 | samlyukov | Note Added: 0001108 | |
2008-01-09 10:55 | vskirdin | Note Added: 0001111 | |
2008-01-09 11:19 | samlyukov | Status | feedback => resolved |
2008-01-09 11:19 | samlyukov | Resolution | open => fixed |
2008-01-10 00:12 | vskirdin | Build | => 407 |
2008-01-10 00:12 | vskirdin | Status | resolved => closed |
2008-01-10 00:12 | vskirdin | Fixed in Version | => 1.80 alpha 1 |