From fb374bcb65b70e1aa72d242822de80dc884df836 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Tue, 19 Sep 2023 14:02:12 -0700 Subject: [PATCH 1/4] Update to UnRAR v6.2.12 --- libclamunrar/arcread.cpp | 2 +- libclamunrar/crypt5.cpp | 8 +++++ libclamunrar/dll.cpp | 8 ++--- libclamunrar/dll.rc | 8 ++--- libclamunrar/extinfo.cpp | 67 +++------------------------------- libclamunrar/extinfo.hpp | 1 - libclamunrar/extract.cpp | 76 +++++++++++++++++++-------------------- libclamunrar/filefn.cpp | 62 ++++++++++++++++++++++++++++++++ libclamunrar/filefn.hpp | 2 ++ libclamunrar/unpack.cpp | 10 ++---- libclamunrar/version.hpp | 6 ++-- libclamunrar/win32stm.cpp | 24 +++++++++++++ 12 files changed, 152 insertions(+), 122 deletions(-) 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..178d5595a4 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); @@ -329,7 +329,7 @@ int PASCAL ProcessFile(HANDLE hArcData,int Operation,char *DestPath,char *DestNa { Data->Cmd.DllError=0; if (Data->OpenMode==RAR_OM_LIST || Data->OpenMode==RAR_OM_LIST_INCSPLIT || - Operation==RAR_SKIP) // && !Data->Arc.Solid) + Operation==RAR_SKIP && !Data->Arc.Solid) { if (Data->Arc.Volume && Data->Arc.GetHeaderType()==HEAD_FILE && Data->Arc.FileHead.SplitAfter) @@ -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..f2eb166572 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 @@ -82,7 +78,7 @@ void CmdExtract::DoExtract() { if (Cmd->ManualPassword) Cmd->Password.Clean(); // Clean user entered password before processing next archive. - + ReconstructDone=false; // Must be reset here, not in ExtractArchiveInit(). UseExactVolName=false; // Must be reset here, not in ExtractArchiveInit(). while (true) @@ -98,7 +94,7 @@ void CmdExtract::DoExtract() if (Cmd->ManualPassword) Cmd->Password.Clean(); - if (TotalFileCount==0 && Cmd->Command[0]!='I' && + if (TotalFileCount==0 && Cmd->Command[0]!='I' && ErrHandler.GetErrorCode()!=RARX_BADPWD) // Not in case of wrong archive password. { if (!PasswordCancelled) @@ -240,7 +236,7 @@ EXTRACT_ARC_CODE CmdExtract::ExtractArchive() return EXTRACT_ARC_REPEAT; } #endif - + // Calculate the total size of all accessible volumes. // This size is necessary to display the correct total progress indicator. @@ -249,7 +245,7 @@ EXTRACT_ARC_CODE CmdExtract::ExtractArchive() while (true) { - // First volume is already added to DataIO.TotalArcSize + // First volume is already added to DataIO.TotalArcSize // in initial TotalArcSize calculation in DoExtract. // So we skip it and start from second volume. NextVolumeName(NextName,ASIZE(NextName),!Arc.NewNumbering); @@ -494,7 +490,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) if (wcscmp(ArcFileName,RefList[I].RefName)==0) { ExtractRef *MatchedRef=&RefList[I]; - + if (!Cmd->Test) // While harmless, it is useless for 't'. { // If reference source isn't selected, but target is selected, @@ -513,13 +509,13 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) RefTarget=true; // Need it even for 't' to test the reference source. break; } - + if (Arc.FileHead.Encrypted && Cmd->SkipEncrypted) if (Arc.Solid) return false; // Abort the entire extraction for solid archive. else MatchFound=false; // Skip only the current file for non-solid archive. - + if (MatchFound || RefTarget || (SkipSolid=Arc.Solid)!=0) { // First common call of uiStartFileExtract. It is done before overwrite @@ -541,10 +537,10 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) { if (FD.mtime >= Arc.FileHead.mtime) { - // If directory already exists and its modification time is newer - // than start of extraction, it is likely it was created - // when creating a path to one of already extracted items. - // In such case we'll better update its time even if archived + // If directory already exists and its modification time is newer + // than start of extraction, it is likely it was created + // when creating a path to one of already extracted items. + // In such case we'll better update its time even if archived // directory is older. if (!FD.IsDir || FD.mtimeDllDestName,ASIZE(DestFileName)); #endif + if (ExtrFile && Command!='P' && !Cmd->Test && !Cmd->AbsoluteLinks && + ConvertSymlinkPaths) + ExtrFile=LinksToDirs(DestFileName,Cmd->ExtrPath,LastCheckedSymlink); + File CurFile; bool LinkEntry=Arc.FileHead.RedirType!=FSREDIR_NONE; @@ -684,7 +684,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) ExtrFile=true; // We changed SkipSolid, so we need to call uiStartFileExtract - // with "Skip" parameter to change the operation status + // with "Skip" parameter to change the operation status // from "extracting" to "skipping". For example, it can be necessary // if user answered "No" to overwrite prompt when unpacking // a solid archive. @@ -775,7 +775,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) if (Type==FSREDIR_HARDLINK || Type==FSREDIR_FILECOPY) { wchar RedirName[NM]; - + // 2022.11.15: Might be needed when unpacking WinRAR 5.0 links with // Unix RAR. WinRAR 5.0 used \ path separators here, when beginning // from 5.10 even Windows version uses / internally and converts @@ -806,7 +806,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) // Unix symlink can have its own owner data. if (LinkSuccess) SetFileHeaderExtra(Cmd,Arc,DestFileName); - + ConvertSymlinkPaths|=LinkSuccess && UpLink; // We do not actually need to reset the cache here if we cache @@ -826,7 +826,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) uiMsg(UIERROR_UNKNOWNEXTRA,Arc.FileName,ArcFileName); LinkSuccess=false; } - + if (!LinkSuccess || Arc.Format==RARFMT15 && !FileCreateMode) { // RAR 5.x links have a valid data checksum even in case of @@ -874,9 +874,9 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) else if (Arc.FileHead.Method!=0 && Arc.FileHead.UnpSize>0 && ValidCRC) AnySolidDataUnpackedWell=true; - + bool BrokenFile=false; - + // Checksum is not calculated in skip solid mode for performance reason. if (!SkipSolid && ShowChecksum) { @@ -888,7 +888,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) } else { - if (Arc.FileHead.Encrypted && (!Arc.FileHead.UsePswCheck || + if (Arc.FileHead.Encrypted && (!Arc.FileHead.UsePswCheck || Arc.BrokenHeader) && !AnySolidDataUnpackedWell) uiMsg(UIERROR_CHECKSUMENC,Arc.FileName,ArcFileName); else @@ -909,7 +909,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) // We check SkipSolid to remove percent for skipped solid files only. // We must not apply these \b to links with ShowChecksum==false // and their possible error messages. - if (SkipSolid) + if (SkipSolid) mprintf(L"\b\b\b\b\b "); } @@ -922,7 +922,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) bool SetAttrOnly=LinkEntry && Arc.FileHead.RedirType==FSREDIR_HARDLINK && LinkSuccess; if (!TestMode && (Command=='X' || Command=='E') && - (!LinkEntry || SetAttrOnly || Arc.FileHead.RedirType==FSREDIR_FILECOPY && LinkSuccess) && + (!LinkEntry || SetAttrOnly || Arc.FileHead.RedirType==FSREDIR_FILECOPY && LinkSuccess) && (!BrokenFile || Cmd->KeepBroken)) { // Below we use DestFileName instead of CurFile.FileName, @@ -949,7 +949,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) Cmd->xmtime==EXTTIME_NONE ? NULL:&Arc.FileHead.mtime, Cmd->xatime==EXTTIME_NONE ? NULL:&Arc.FileHead.atime); } - + #if defined(_WIN_ALL) && !defined(SFX_MODULE) if (Cmd->SetCompressedAttr && (Arc.FileHead.FileAttr & FILE_ATTRIBUTE_COMPRESSED)!=0) @@ -1094,7 +1094,7 @@ void CmdExtract::ExtrPrepareName(Archive &Arc,const wchar *ArcFileName,wchar *De wcsncpyz(DestName,ArcFileName,DestSize); return; } - + wcsncpyz(DestName,Cmd->ExtrPath,DestSize); if (*Cmd->ExtrPath!=0) @@ -1141,7 +1141,7 @@ void CmdExtract::ExtrPrepareName(Archive &Arc,const wchar *ArcFileName,wchar *De { size_t NameLength=wcslen(ArcFileName); if (NameLength>=ArcPathLength && wcsnicompc(ArcPath,ArcFileName,ArcPathLength)==0 && - (IsPathDiv(ArcPath[ArcPathLength-1]) || + (IsPathDiv(ArcPath[ArcPathLength-1]) || IsPathDiv(ArcFileName[ArcPathLength]) || ArcFileName[ArcPathLength]==0)) { ArcFileName+=Min(ArcPathLength,NameLength); @@ -1460,12 +1460,12 @@ bool CmdExtract::CheckUnpVer(Archive &Arc,const wchar *ArcFileName) // Find non-matched reference sources in solid and non-solid archives. // Detect the optimal start position for semi-solid archives // and optimal start volume for independent solid volumes. -// +// // Alternatively we could collect references while extracting an archive // and perform the second extraction pass for references only. // But it would be slower for solid archives than scaning headers // in first pass and extracting everything in second, as implemented now. -// +// void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumbering) { FreeAnalyzeData(); // If processing non-first archive in multiple archives set. @@ -1481,13 +1481,13 @@ void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumberi GetFirstVolIfFullSet(ArcName,NewNumbering,NextName,ASIZE(NextName)); else wcsncpyz(NextName,ArcName,ASIZE(NextName)); - + bool MatchFound=false; bool PrevMatched=false; bool OpenNext=false; bool FirstVolume=true; - + // We shall set FirstFile once for all volumes and not for each volume. // So we do not reuse the outdated Analyze->StartPos from previous volume // if extracted file resides completely in the beginning of current one. @@ -1542,7 +1542,7 @@ void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumberi wcsncpyz(Analyze->StartName,NextName,ASIZE(Analyze->StartName)); // We shall set FirstFile once for all volumes for this code - // to work properly. Alternatively we could append + // to work properly. Alternatively we could append // "|| Analyze->StartPos!=0" to the condition, so we do not reuse // the outdated Analyze->StartPos value from previous volume. if (!FirstFile) 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/unpack.cpp b/libclamunrar/unpack.cpp index f82bd4753b..9236e748bc 100644 --- a/libclamunrar/unpack.cpp +++ b/libclamunrar/unpack.cpp @@ -91,12 +91,6 @@ void Unpack::Init(size_t WinSize,bool Solid) if ((WinSize>>16)>0x10000) // Window size must not exceed 4 GB. return; - // Unrar does not support window size greather than 1GB at this time. - // Any request for a window larger than 1GB should be ignored. - const size_t MaxAllocSize=0x40000000; - if (WinSize>MaxAllocSize) - WinSize=MaxAllocSize; - // Archiving code guarantees that window size does not grow in the same // solid stream. So if we are here, we are either creating a new window // or increasing the size of non-solid window. So we could safely reject @@ -271,7 +265,7 @@ void Unpack::MakeDecodeTables(byte *LengthTable,DecodeTable *Dec,uint Size) Dec->DecodeLen[I]=(uint)LeftAligned; // Every item of this array contains the sum of all preceding items. - // So it contains the start position in code list for every bit length. + // So it contains the start position in code list for every bit length. Dec->DecodePos[I]=Dec->DecodePos[I-1]+LengthCount[I-1]; } @@ -334,7 +328,7 @@ void Unpack::MakeDecodeTables(byte *LengthTable,DecodeTable *Dec,uint Size) uint BitField=Code<<(16-Dec->QuickBits); // Prepare the table for quick decoding of bit lengths. - + // Find the upper limit for current bit field and adjust the bit length // accordingly if necessary. while (CurBitLengthDecodeLen) && BitField>=Dec->DecodeLen[CurBitLength]) 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); From e4ce6836e0c124a875b3c9bf139f4968b477f06e Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Sun, 18 Jul 2021 18:26:23 -0700 Subject: [PATCH 2/4] Patch UnRAR: limit dict winsize to 1GB Prevent allocating more than 1GB regardless of what is requested. RAR dictionary sizes may not be larger than 1GB, at least in the current version. This is a cherry-pick of commit 9b444e7e02639d1030bbc38f9c95511bbe19e67b --- libclamunrar/unpack.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libclamunrar/unpack.cpp b/libclamunrar/unpack.cpp index 9236e748bc..f82bd4753b 100644 --- a/libclamunrar/unpack.cpp +++ b/libclamunrar/unpack.cpp @@ -91,6 +91,12 @@ void Unpack::Init(size_t WinSize,bool Solid) if ((WinSize>>16)>0x10000) // Window size must not exceed 4 GB. return; + // Unrar does not support window size greather than 1GB at this time. + // Any request for a window larger than 1GB should be ignored. + const size_t MaxAllocSize=0x40000000; + if (WinSize>MaxAllocSize) + WinSize=MaxAllocSize; + // Archiving code guarantees that window size does not grow in the same // solid stream. So if we are here, we are either creating a new window // or increasing the size of non-solid window. So we could safely reject @@ -265,7 +271,7 @@ void Unpack::MakeDecodeTables(byte *LengthTable,DecodeTable *Dec,uint Size) Dec->DecodeLen[I]=(uint)LeftAligned; // Every item of this array contains the sum of all preceding items. - // So it contains the start position in code list for every bit length. + // So it contains the start position in code list for every bit length. Dec->DecodePos[I]=Dec->DecodePos[I-1]+LengthCount[I-1]; } @@ -328,7 +334,7 @@ void Unpack::MakeDecodeTables(byte *LengthTable,DecodeTable *Dec,uint Size) uint BitField=Code<<(16-Dec->QuickBits); // Prepare the table for quick decoding of bit lengths. - + // Find the upper limit for current bit field and adjust the bit length // accordingly if necessary. while (CurBitLengthDecodeLen) && BitField>=Dec->DecodeLen[CurBitLength]) From 4520ad7b89f95d5fc300787ae9236fcfc8c0c292 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Fri, 28 Sep 2018 14:30:42 -0400 Subject: [PATCH 3/4] Patch UnRAR: allow skipping files in solid archives This is a cherry-pick of commit 24f225c21ffa77dbd4b2a3c0377b9b3f86547ce6 Modification to unrar codebase allowing skipping of files within Solid archives when parsing in extraction mode, enabling us to skip encrypted files while still scanning metadata and potentially scanning unencrypted files later in the archive. --- libclamunrar/dll.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libclamunrar/dll.cpp b/libclamunrar/dll.cpp index 178d5595a4..6661ce334c 100644 --- a/libclamunrar/dll.cpp +++ b/libclamunrar/dll.cpp @@ -329,7 +329,7 @@ int PASCAL ProcessFile(HANDLE hArcData,int Operation,char *DestPath,char *DestNa { Data->Cmd.DllError=0; if (Data->OpenMode==RAR_OM_LIST || Data->OpenMode==RAR_OM_LIST_INCSPLIT || - Operation==RAR_SKIP && !Data->Arc.Solid) + Operation==RAR_SKIP) // && !Data->Arc.Solid) { if (Data->Arc.Volume && Data->Arc.GetHeaderType()==HEAD_FILE && Data->Arc.FileHead.SplitAfter) From 7749c2c99eee4eb8426507cc8836cb8e26627b24 Mon Sep 17 00:00:00 2001 From: Micah Snyder Date: Wed, 23 Aug 2023 10:06:59 -0700 Subject: [PATCH 4/4] Patch UnRAR: don't replace symlinks with directories UnRAR logic replaces directory symlinks found within archive file entry file paths with actual directories by deleting them after they're extracted. Unfortunately, this logic extends to deleting existing directories if you set the `DestName` instead of the `DestPath` in this API: rc = RARProcessFile(hArchive, RAR_EXTRACT, NULL, destFilePath); In the future UnRAR may change to disable the `LinksToDirs()` feature if using the `DestName` parameter. In the meantime, this commit completely disables it for our use case. --- libclamunrar/extract.cpp | 60 +++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/libclamunrar/extract.cpp b/libclamunrar/extract.cpp index f2eb166572..51d81476be 100644 --- a/libclamunrar/extract.cpp +++ b/libclamunrar/extract.cpp @@ -78,7 +78,7 @@ void CmdExtract::DoExtract() { if (Cmd->ManualPassword) Cmd->Password.Clean(); // Clean user entered password before processing next archive. - + ReconstructDone=false; // Must be reset here, not in ExtractArchiveInit(). UseExactVolName=false; // Must be reset here, not in ExtractArchiveInit(). while (true) @@ -94,7 +94,7 @@ void CmdExtract::DoExtract() if (Cmd->ManualPassword) Cmd->Password.Clean(); - if (TotalFileCount==0 && Cmd->Command[0]!='I' && + if (TotalFileCount==0 && Cmd->Command[0]!='I' && ErrHandler.GetErrorCode()!=RARX_BADPWD) // Not in case of wrong archive password. { if (!PasswordCancelled) @@ -236,7 +236,7 @@ EXTRACT_ARC_CODE CmdExtract::ExtractArchive() return EXTRACT_ARC_REPEAT; } #endif - + // Calculate the total size of all accessible volumes. // This size is necessary to display the correct total progress indicator. @@ -245,7 +245,7 @@ EXTRACT_ARC_CODE CmdExtract::ExtractArchive() while (true) { - // First volume is already added to DataIO.TotalArcSize + // First volume is already added to DataIO.TotalArcSize // in initial TotalArcSize calculation in DoExtract. // So we skip it and start from second volume. NextVolumeName(NextName,ASIZE(NextName),!Arc.NewNumbering); @@ -490,7 +490,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) if (wcscmp(ArcFileName,RefList[I].RefName)==0) { ExtractRef *MatchedRef=&RefList[I]; - + if (!Cmd->Test) // While harmless, it is useless for 't'. { // If reference source isn't selected, but target is selected, @@ -509,13 +509,13 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) RefTarget=true; // Need it even for 't' to test the reference source. break; } - + if (Arc.FileHead.Encrypted && Cmd->SkipEncrypted) if (Arc.Solid) return false; // Abort the entire extraction for solid archive. else MatchFound=false; // Skip only the current file for non-solid archive. - + if (MatchFound || RefTarget || (SkipSolid=Arc.Solid)!=0) { // First common call of uiStartFileExtract. It is done before overwrite @@ -537,10 +537,10 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) { if (FD.mtime >= Arc.FileHead.mtime) { - // If directory already exists and its modification time is newer - // than start of extraction, it is likely it was created - // when creating a path to one of already extracted items. - // In such case we'll better update its time even if archived + // If directory already exists and its modification time is newer + // than start of extraction, it is likely it was created + // when creating a path to one of already extracted items. + // In such case we'll better update its time even if archived // directory is older. if (!FD.IsDir || FD.mtimeDllDestName,ASIZE(DestFileName)); #endif - if (ExtrFile && Command!='P' && !Cmd->Test && !Cmd->AbsoluteLinks && - ConvertSymlinkPaths) - ExtrFile=LinksToDirs(DestFileName,Cmd->ExtrPath,LastCheckedSymlink); - File CurFile; bool LinkEntry=Arc.FileHead.RedirType!=FSREDIR_NONE; @@ -684,7 +680,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) ExtrFile=true; // We changed SkipSolid, so we need to call uiStartFileExtract - // with "Skip" parameter to change the operation status + // with "Skip" parameter to change the operation status // from "extracting" to "skipping". For example, it can be necessary // if user answered "No" to overwrite prompt when unpacking // a solid archive. @@ -775,7 +771,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) if (Type==FSREDIR_HARDLINK || Type==FSREDIR_FILECOPY) { wchar RedirName[NM]; - + // 2022.11.15: Might be needed when unpacking WinRAR 5.0 links with // Unix RAR. WinRAR 5.0 used \ path separators here, when beginning // from 5.10 even Windows version uses / internally and converts @@ -806,7 +802,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) // Unix symlink can have its own owner data. if (LinkSuccess) SetFileHeaderExtra(Cmd,Arc,DestFileName); - + ConvertSymlinkPaths|=LinkSuccess && UpLink; // We do not actually need to reset the cache here if we cache @@ -826,7 +822,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) uiMsg(UIERROR_UNKNOWNEXTRA,Arc.FileName,ArcFileName); LinkSuccess=false; } - + if (!LinkSuccess || Arc.Format==RARFMT15 && !FileCreateMode) { // RAR 5.x links have a valid data checksum even in case of @@ -874,9 +870,9 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) else if (Arc.FileHead.Method!=0 && Arc.FileHead.UnpSize>0 && ValidCRC) AnySolidDataUnpackedWell=true; - + bool BrokenFile=false; - + // Checksum is not calculated in skip solid mode for performance reason. if (!SkipSolid && ShowChecksum) { @@ -888,7 +884,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) } else { - if (Arc.FileHead.Encrypted && (!Arc.FileHead.UsePswCheck || + if (Arc.FileHead.Encrypted && (!Arc.FileHead.UsePswCheck || Arc.BrokenHeader) && !AnySolidDataUnpackedWell) uiMsg(UIERROR_CHECKSUMENC,Arc.FileName,ArcFileName); else @@ -909,7 +905,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) // We check SkipSolid to remove percent for skipped solid files only. // We must not apply these \b to links with ShowChecksum==false // and their possible error messages. - if (SkipSolid) + if (SkipSolid) mprintf(L"\b\b\b\b\b "); } @@ -922,7 +918,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) bool SetAttrOnly=LinkEntry && Arc.FileHead.RedirType==FSREDIR_HARDLINK && LinkSuccess; if (!TestMode && (Command=='X' || Command=='E') && - (!LinkEntry || SetAttrOnly || Arc.FileHead.RedirType==FSREDIR_FILECOPY && LinkSuccess) && + (!LinkEntry || SetAttrOnly || Arc.FileHead.RedirType==FSREDIR_FILECOPY && LinkSuccess) && (!BrokenFile || Cmd->KeepBroken)) { // Below we use DestFileName instead of CurFile.FileName, @@ -949,7 +945,7 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) Cmd->xmtime==EXTTIME_NONE ? NULL:&Arc.FileHead.mtime, Cmd->xatime==EXTTIME_NONE ? NULL:&Arc.FileHead.atime); } - + #if defined(_WIN_ALL) && !defined(SFX_MODULE) if (Cmd->SetCompressedAttr && (Arc.FileHead.FileAttr & FILE_ATTRIBUTE_COMPRESSED)!=0) @@ -1094,7 +1090,7 @@ void CmdExtract::ExtrPrepareName(Archive &Arc,const wchar *ArcFileName,wchar *De wcsncpyz(DestName,ArcFileName,DestSize); return; } - + wcsncpyz(DestName,Cmd->ExtrPath,DestSize); if (*Cmd->ExtrPath!=0) @@ -1141,7 +1137,7 @@ void CmdExtract::ExtrPrepareName(Archive &Arc,const wchar *ArcFileName,wchar *De { size_t NameLength=wcslen(ArcFileName); if (NameLength>=ArcPathLength && wcsnicompc(ArcPath,ArcFileName,ArcPathLength)==0 && - (IsPathDiv(ArcPath[ArcPathLength-1]) || + (IsPathDiv(ArcPath[ArcPathLength-1]) || IsPathDiv(ArcFileName[ArcPathLength]) || ArcFileName[ArcPathLength]==0)) { ArcFileName+=Min(ArcPathLength,NameLength); @@ -1460,12 +1456,12 @@ bool CmdExtract::CheckUnpVer(Archive &Arc,const wchar *ArcFileName) // Find non-matched reference sources in solid and non-solid archives. // Detect the optimal start position for semi-solid archives // and optimal start volume for independent solid volumes. -// +// // Alternatively we could collect references while extracting an archive // and perform the second extraction pass for references only. // But it would be slower for solid archives than scaning headers // in first pass and extracting everything in second, as implemented now. -// +// void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumbering) { FreeAnalyzeData(); // If processing non-first archive in multiple archives set. @@ -1481,13 +1477,13 @@ void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumberi GetFirstVolIfFullSet(ArcName,NewNumbering,NextName,ASIZE(NextName)); else wcsncpyz(NextName,ArcName,ASIZE(NextName)); - + bool MatchFound=false; bool PrevMatched=false; bool OpenNext=false; bool FirstVolume=true; - + // We shall set FirstFile once for all volumes and not for each volume. // So we do not reuse the outdated Analyze->StartPos from previous volume // if extracted file resides completely in the beginning of current one. @@ -1542,7 +1538,7 @@ void CmdExtract::AnalyzeArchive(const wchar *ArcName,bool Volume,bool NewNumberi wcsncpyz(Analyze->StartName,NextName,ASIZE(Analyze->StartName)); // We shall set FirstFile once for all volumes for this code - // to work properly. Alternatively we could append + // to work properly. Alternatively we could append // "|| Analyze->StartPos!=0" to the condition, so we do not reuse // the outdated Analyze->StartPos value from previous volume. if (!FirstFile)