diff --git a/libclamunrar/arcread.cpp b/libclamunrar/arcread.cpp index f5ef9aa517..6b7dd98ee6 100644 --- a/libclamunrar/arcread.cpp +++ b/libclamunrar/arcread.cpp @@ -991,7 +991,7 @@ void Archive::ProcessExtra50(RawRead *Raw,size_t ExtraSize,const BaseBlock *bb) if ((Flags & MHEXTRA_METADATA_NAME)!=0) { uint64 NameSize=Raw->GetV(); - if (NameSize<0x10000) // Prevent excessive allocation. + if (NameSize>0 && NameSize<0x10000) // Prevent excessive allocation. { std::vector NameU((size_t)NameSize); // UTF-8 name. Raw->GetB(&NameU[0],(size_t)NameSize); diff --git a/libclamunrar/crypt5.cpp b/libclamunrar/crypt5.cpp index 5ed65af811..bb9b2bab2c 100644 --- a/libclamunrar/crypt5.cpp +++ b/libclamunrar/crypt5.cpp @@ -133,7 +133,15 @@ void CryptData::SetKey50(bool Encrypt,SecPassword *Password,const wchar *PwdW, byte *PswCheck) { if (Lg2Cnt>CRYPT5_KDF_LG2_COUNT_MAX) + { + // Initialize these fields to prevent uninitialized data access warnings + // by analyzing tools when accessing returned data. + if (HashKey!=nullptr) + memset(HashKey,0,SHA256_DIGEST_SIZE); + if (PswCheck!=nullptr) + memset(PswCheck,0,SIZE_PSWCHECK); return; + } byte Key[32],PswCheckValue[SHA256_DIGEST_SIZE],HashKeyValue[SHA256_DIGEST_SIZE]; bool Found=false; diff --git a/libclamunrar/dll.cpp b/libclamunrar/dll.cpp index 0464c9ace6..6661ce334c 100644 --- a/libclamunrar/dll.cpp +++ b/libclamunrar/dll.cpp @@ -242,10 +242,10 @@ int PASCAL RARReadHeaderEx(HANDLE hArcData,struct RARHeaderDataEx *D) else return Code; } - wcsncpy(D->ArcNameW,Data->Arc.FileName,ASIZE(D->ArcNameW)); + wcsncpyz(D->ArcNameW,Data->Arc.FileName,ASIZE(D->ArcNameW)); WideToChar(D->ArcNameW,D->ArcName,ASIZE(D->ArcName)); - wcsncpy(D->FileNameW,hd->FileName,ASIZE(D->FileNameW)); + wcsncpyz(D->FileNameW,hd->FileName,ASIZE(D->FileNameW)); WideToChar(D->FileNameW,D->FileName,ASIZE(D->FileName)); #ifdef _WIN_ALL CharToOemA(D->FileName,D->FileName); @@ -377,7 +377,7 @@ int PASCAL ProcessFile(HANDLE hArcData,int Operation,char *DestPath,char *DestNa if (DestPathW!=NULL) { - wcsncpy(Data->Cmd.ExtrPath,DestPathW,ASIZE(Data->Cmd.ExtrPath)); + wcsncpyz(Data->Cmd.ExtrPath,DestPathW,ASIZE(Data->Cmd.ExtrPath)); AddEndSlash(Data->Cmd.ExtrPath,ASIZE(Data->Cmd.ExtrPath)); } diff --git a/libclamunrar/dll.rc b/libclamunrar/dll.rc index 3ca50eb1f7..b7f07785b1 100644 --- a/libclamunrar/dll.rc +++ b/libclamunrar/dll.rc @@ -2,8 +2,8 @@ #include VS_VERSION_INFO VERSIONINFO -FILEVERSION 6, 23, 100, 944 -PRODUCTVERSION 6, 23, 100, 944 +FILEVERSION 6, 24, 100, 1007 +PRODUCTVERSION 6, 24, 100, 1007 FILEOS VOS__WINDOWS32 FILETYPE VFT_APP { @@ -14,8 +14,8 @@ FILETYPE VFT_APP VALUE "CompanyName", "Alexander Roshal\0" VALUE "ProductName", "RAR decompression library\0" VALUE "FileDescription", "RAR decompression library\0" - VALUE "FileVersion", "6.23.0\0" - VALUE "ProductVersion", "6.23.0\0" + VALUE "FileVersion", "6.24.0\0" + VALUE "ProductVersion", "6.24.0\0" VALUE "LegalCopyright", "Copyright © Alexander Roshal 1993-2023\0" VALUE "OriginalFilename", "Unrar.dll\0" } diff --git a/libclamunrar/extinfo.cpp b/libclamunrar/extinfo.cpp index 4bf8046a74..d61747e1aa 100644 --- a/libclamunrar/extinfo.cpp +++ b/libclamunrar/extinfo.cpp @@ -80,10 +80,13 @@ static int CalcAllowedDepth(const wchar *Name) bool Dot2=Name[1]=='.' && Name[2]=='.' && (IsPathDiv(Name[3]) || Name[3]==0); if (!Dot && !Dot2) AllowedDepth++; + else + if (Dot2) + AllowedDepth--; } Name++; } - return AllowedDepth; + return AllowedDepth < 0 ? 0 : AllowedDepth; } @@ -106,68 +109,6 @@ static bool LinkInPath(const wchar *Name) } -// Delete symbolic links in file path, if any, and replace them by directories. -// Prevents extracting files outside of destination folder with symlink chains. -bool LinksToDirs(const wchar *SrcName,const wchar *SkipPart,std::wstring &LastChecked) -{ - // Unlike Unix, Windows doesn't expand lnk1 in symlink targets like - // "lnk1/../dir", but converts the path to "dir". In Unix we need to call - // this function to prevent placing unpacked files outside of destination - // folder if previously we unpacked "dir/lnk1" -> "..", - // "dir/lnk2" -> "lnk1/.." and "dir/lnk2/anypath/poc.txt". - // We may still need this function to prevent abusing symlink chains - // in link source path if we remove detection of such chains - // in IsRelativeSymlinkSafe. This function seems to make other symlink - // related safety checks redundant, but for now we prefer to keep them too. - // - // 2022.12.01: the performance impact is minimized after adding the check - // against the previous path and enabling this verification only after - // extracting a symlink with ".." in target. So we enabled it for Windows - // as well for extra safety. -//#ifdef _UNIX - wchar Path[NM]; - if (wcslen(SrcName)>=ASIZE(Path)) - return false; // It should not be that long, skip. - wcsncpyz(Path,SrcName,ASIZE(Path)); - - size_t SkipLength=wcslen(SkipPart); - - if (SkipLength>0 && wcsncmp(Path,SkipPart,SkipLength)!=0) - SkipLength=0; // Parameter validation, not really needed now. - - // Do not check parts already checked in previous path to improve performance. - for (uint I=0;Path[I]!=0 && ISkipLength) - SkipLength=I; - - wchar *Name=Path; - if (SkipLength>0) - { - // Avoid converting symlinks in destination path part specified by user. - Name+=SkipLength; - while (IsPathDiv(*Name)) - Name++; - } - - for (wchar *s=Path+wcslen(Path)-1;s>Name;s--) - if (IsPathDiv(*s)) - { - *s=0; - FindData FD; - if (FindFile::FastFind(Path,&FD,true) && FD.IsLink) -#ifdef _WIN_ALL - if (!DelDir(Path)) -#else - if (!DelFile(Path)) -#endif - return false; // Couldn't delete the symlink to replace it with directory. - } - LastChecked=SrcName; -//#endif - return true; -} - - bool IsRelativeSymlinkSafe(CommandData *Cmd,const wchar *SrcName,const wchar *PrepSrcName,const wchar *TargetName) { // Catch root dir based /path/file paths also as stuff like \\?\. diff --git a/libclamunrar/extinfo.hpp b/libclamunrar/extinfo.hpp index d8551d463d..da82ec339e 100644 --- a/libclamunrar/extinfo.hpp +++ b/libclamunrar/extinfo.hpp @@ -1,7 +1,6 @@ #ifndef _RAR_EXTINFO_ #define _RAR_EXTINFO_ -bool LinksToDirs(const wchar *SrcName,const wchar *SkipPart,std::wstring &LastChecked); bool IsRelativeSymlinkSafe(CommandData *Cmd,const wchar *SrcName,const wchar *PrepSrcName,const wchar *TargetName); bool ExtractSymlink(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const wchar *LinkName,bool &UpLink); #ifdef _UNIX diff --git a/libclamunrar/extract.cpp b/libclamunrar/extract.cpp index 4b746fc020..51d81476be 100644 --- a/libclamunrar/extract.cpp +++ b/libclamunrar/extract.cpp @@ -14,20 +14,16 @@ CmdExtract::CmdExtract(CommandData *Cmd) TotalFileCount=0; // Common for all archives involved. Set here instead of DoExtract() - // to use in unrar.dll too. Allows to avoid LinksToDirs() calls - // and save CPU time in no symlinks including ".." in target were extracted. -#if defined(_WIN_ALL) - // We can't expand symlink path components in another symlink target - // in Windows. We can't create symlinks in Android now. Even though we do not - // really need LinksToDirs() calls in these systems, we still call it - // for extra safety, but only if symlink with ".." in target was extracted. - ConvertSymlinkPaths=false; -#else + // to use in unrar.dll too. // We enable it by default in Unix to care about the case when several // archives are unpacked to same directory with several independent RAR runs. // Worst case performance penalty for a lot of small files seems to be ~3%. + // 2023.09.15: Windows performance impact seems to be negligible, + // less than 0.5% when extracting mix of small files and folders. + // So for extra security we enabled it for Windows too, even though + // unlike Unix, Windows doesn't expand lnk1 in symlink targets like + // "lnk1/../dir", but converts such path to "dir". ConvertSymlinkPaths=true; -#endif Unp=new Unpack(&DataIO); #ifdef RAR_SMP diff --git a/libclamunrar/filefn.cpp b/libclamunrar/filefn.cpp index aaef305b8e..4bcc8bfd47 100644 --- a/libclamunrar/filefn.cpp +++ b/libclamunrar/filefn.cpp @@ -542,3 +542,65 @@ void ResetFileCache(const wchar *Name) + + +// Delete symbolic links in file path, if any, and replace them by directories. +// Prevents extracting files outside of destination folder with symlink chains. +bool LinksToDirs(const wchar *SrcName,const wchar *SkipPart,std::wstring &LastChecked) +{ + // Unlike Unix, Windows doesn't expand lnk1 in symlink targets like + // "lnk1/../dir", but converts the path to "dir". In Unix we need to call + // this function to prevent placing unpacked files outside of destination + // folder if previously we unpacked "dir/lnk1" -> "..", + // "dir/lnk2" -> "lnk1/.." and "dir/lnk2/anypath/poc.txt". + // We may still need this function to prevent abusing symlink chains + // in link source path if we remove detection of such chains + // in IsRelativeSymlinkSafe. This function seems to make other symlink + // related safety checks redundant, but for now we prefer to keep them too. + // + // 2022.12.01: the performance impact is minimized after adding the check + // against the previous path and enabling this verification only after + // extracting a symlink with ".." in target. So we enabled it for Windows + // as well for extra safety. +//#ifdef _UNIX + wchar Path[NM]; + if (wcslen(SrcName)>=ASIZE(Path)) + return false; // It should not be that long, skip. + wcsncpyz(Path,SrcName,ASIZE(Path)); + + size_t SkipLength=wcslen(SkipPart); + + if (SkipLength>0 && wcsncmp(Path,SkipPart,SkipLength)!=0) + SkipLength=0; // Parameter validation, not really needed now. + + // Do not check parts already checked in previous path to improve performance. + for (uint I=0;Path[I]!=0 && ISkipLength) + SkipLength=I; + + wchar *Name=Path; + if (SkipLength>0) + { + // Avoid converting symlinks in destination path part specified by user. + Name+=SkipLength; + while (IsPathDiv(*Name)) + Name++; + } + + for (wchar *s=Path+wcslen(Path)-1;s>Name;s--) + if (IsPathDiv(*s)) + { + *s=0; + FindData FD; + if (FindFile::FastFind(Path,&FD,true) && FD.IsLink) +#ifdef _WIN_ALL + if (!DelDir(Path)) +#else + if (!DelFile(Path)) +#endif + return false; // Couldn't delete the symlink to replace it with directory. + } + LastChecked=SrcName; +//#endif + return true; +} diff --git a/libclamunrar/filefn.hpp b/libclamunrar/filefn.hpp index 53d86653fe..d73d55e1a3 100644 --- a/libclamunrar/filefn.hpp +++ b/libclamunrar/filefn.hpp @@ -46,4 +46,6 @@ void ResetFileCache(const wchar *Name); +bool LinksToDirs(const wchar *SrcName,const wchar *SkipPart,std::wstring &LastChecked); + #endif diff --git a/libclamunrar/version.hpp b/libclamunrar/version.hpp index 1f45c3a189..c9fffcaca4 100644 --- a/libclamunrar/version.hpp +++ b/libclamunrar/version.hpp @@ -1,6 +1,6 @@ #define RARVER_MAJOR 6 -#define RARVER_MINOR 23 +#define RARVER_MINOR 24 #define RARVER_BETA 0 -#define RARVER_DAY 1 -#define RARVER_MONTH 8 +#define RARVER_DAY 3 +#define RARVER_MONTH 10 #define RARVER_YEAR 2023 diff --git a/libclamunrar/win32stm.cpp b/libclamunrar/win32stm.cpp index 9dab728bd4..3b77d2a452 100644 --- a/libclamunrar/win32stm.cpp +++ b/libclamunrar/win32stm.cpp @@ -1,5 +1,23 @@ +#ifdef _WIN_ALL +// StreamName must include the leading ':'. +static bool IsNtfsReservedStream(const wchar *StreamName) +{ + const wchar *Reserved[]{ + L"::$ATTRIBUTE_LIST",L"::$BITMAP",L"::$DATA",L"::$EA",L"::$EA_INFORMATION", + L"::$FILE_NAME",L"::$INDEX_ALLOCATION",L":$I30:$INDEX_ALLOCATION", + L"::$INDEX_ROOT",L"::$LOGGED_UTILITY_STREAM",L":$EFS:$LOGGED_UTILITY_STREAM", + L":$TXF_DATA:$LOGGED_UTILITY_STREAM",L"::$OBJECT_ID",L"::$REPARSE_POINT" + }; + for (const wchar *Name : Reserved) + if (wcsicomp(StreamName,Name)==0) + return true; + return false; +} +#endif + + #if !defined(SFX_MODULE) && defined(_WIN_ALL) void ExtractStreams20(Archive &Arc,const wchar *FileName) { @@ -40,6 +58,9 @@ void ExtractStreams20(Archive &Arc,const wchar *FileName) ConvertPath(StoredName+1,StoredName+1,ASIZE(StoredName)-1); + if (IsNtfsReservedStream(StoredName)) + return; + wcsncatz(StreamName,StoredName,ASIZE(StreamName)); FindData fd; @@ -113,6 +134,9 @@ void ExtractStreams(Archive &Arc,const wchar *FileName,bool TestMode) wcsncatz(FullName,StreamName,ASIZE(FullName)); + if (IsNtfsReservedStream(StreamName)) + return; + FindData fd; bool HostFound=FindFile::FastFind(FileName,&fd);