Package: release.debian.org Severity: normal Tags: bullseye User: release.debian....@packages.debian.org Usertags: pu X-Debbugs-Cc: a...@debian.org
Hi, [ Reason ] unrar-nonfree is affected by CVE-2022-48579 in Bullseye. non-free packages are not supported by the security team but it makes still sense to fix this issue via a point update. [ Impact ] unrar-nonfree would continue to be affected by CVE-2022-48579. [ Tests ] I have manually created a rar archive which includes several symlinks pointing to each other, files with relative paths and special characters and in all cases unrar-nonfree seems to do the right thing. An official reproducer was not available. [ Risks ] If I made a mistake there should be an unpack error or something similar, which is not the case. Command switches didn't change so an external program like xarchiver continues to work as expected. [ Checklist ] [*] *all* changes are documented in the d/changelog [*] I reviewed all changes and I approve them [*] attach debdiff against the package in (old)stable [*] the issue is verified as fixed in unstable [ Other info ] Maintainer approves point update. (#1050080)
diff -Nru unrar-nonfree-6.0.3/debian/changelog unrar-nonfree-6.0.3/debian/changelog --- unrar-nonfree-6.0.3/debian/changelog 2022-05-10 13:26:16.000000000 +0200 +++ unrar-nonfree-6.0.3/debian/changelog 2023-08-20 09:58:26.000000000 +0200 @@ -1,3 +1,13 @@ +unrar-nonfree (1:6.0.3-1+deb11u2) bullseye; urgency=high + + * Non maintainer upload. + * Fix CVE-2022-48579: + It was discovered that UnRAR, an unarchiver for rar files, allows + extraction of files outside of the destination folder via symlink chains. + (Closes: #1050080) + + -- Markus Koschany <a...@debian.org> Sun, 20 Aug 2023 09:58:26 +0200 + unrar-nonfree (1:6.0.3-1+deb11u1) bullseye; urgency=high * Fix CVE-2022-30333 (Closes: #1010837) diff -Nru unrar-nonfree-6.0.3/debian/patches/0013-CVE-2022-48579.patch unrar-nonfree-6.0.3/debian/patches/0013-CVE-2022-48579.patch --- unrar-nonfree-6.0.3/debian/patches/0013-CVE-2022-48579.patch 1970-01-01 01:00:00.000000000 +0100 +++ unrar-nonfree-6.0.3/debian/patches/0013-CVE-2022-48579.patch 2023-08-20 09:58:26.000000000 +0200 @@ -0,0 +1,429 @@ +From: Markus Koschany <a...@debian.org> +Date: Mon, 14 Aug 2023 15:43:54 +0200 +Subject: CVE-2022-48579 + +Origin: https://github.com/pmachapman/unrar/commit/2ecab6bb5ac4f3b88f270218445496662020205f +--- + arcread.cpp | 4 ++- + extinfo.cpp | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---- + extinfo.hpp | 3 +- + extract.cpp | 44 +++++++++++++++++++++++++---- + extract.hpp | 6 ++++ + hardlinks.cpp | 2 -- + model.cpp | 6 ++-- + os.hpp | 1 + + pathfn.cpp | 14 +++++++--- + timefn.hpp | 11 ++++++++ + ulinks.cpp | 6 ++-- + win32stm.cpp | 9 ++++-- + 12 files changed, 170 insertions(+), 25 deletions(-) + +diff --git a/arcread.cpp b/arcread.cpp +index d1df6c0..63858d9 100644 +--- a/arcread.cpp ++++ b/arcread.cpp +@@ -1441,7 +1441,9 @@ bool Archive::ReadSubData(Array<byte> *UnpData,File *DestFile,bool TestMode) + { + if (SubHead.UnpSize>0x1000000) + { +- // So huge allocation must never happen in valid archives. ++ // Prevent the excessive allocation. When reading to memory, normally ++ // this function operates with reasonably small blocks, such as ++ // the archive comment, NTFS ACL or "Zone.Identifier" NTFS stream. + uiMsg(UIERROR_SUBHEADERUNKNOWN,FileName); + return false; + } +diff --git a/extinfo.cpp b/extinfo.cpp +index 5cb90a4..0f25f31 100644 +--- a/extinfo.cpp ++++ b/extinfo.cpp +@@ -112,6 +112,68 @@ 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 && I<LastChecked.size() && Path[I]==LastChecked[I];I++) ++ if (IsPathDiv(Path[I]) && I>SkipLength) ++ 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 \\?\. +@@ -131,10 +193,14 @@ bool IsRelativeSymlinkSafe(CommandData *Cmd,const wchar *SrcName,const wchar *Pr + UpLevels++; + TargetName++; + } +- // If link target includes "..", it must not have another links +- // in the path, because they can bypass our safety check. For example, ++ // If link target includes "..", it must not have another links in its ++ // source path, because they can bypass our safety check. For example, + // suppose we extracted "lnk1" -> "." first and "lnk1/lnk2" -> ".." next +- // or "dir/lnk1" -> ".." first and "dir/lnk1/lnk2" -> ".." next. ++ // or "dir/lnk1" -> ".." first, "dir/lnk1/lnk2" -> ".." next and ++ // file "dir/lnk1/lnk2/poc.txt" last. ++ // Do not confuse with link chains in target, this is in link source path. ++ // It is important for Windows too, though this check can be omitted ++ // if LinksToDirs is invoked in Windows as well. + if (UpLevels>0 && LinkInPath(PrepSrcName)) + return false; + +@@ -160,15 +226,26 @@ bool IsRelativeSymlinkSafe(CommandData *Cmd,const wchar *SrcName,const wchar *Pr + } + + +-bool ExtractSymlink(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const wchar *LinkName) ++bool ExtractSymlink(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const wchar *LinkName,bool &UpLink) + { ++ // Returning true in Uplink indicates that link target might include ".." ++ // and enables additional checks. It is ok to falsely return true here, ++ // as it implies only the minor performance penalty. But we shall always ++ // return true for links with ".." in target for security reason. ++ ++ UpLink=true; // Assume the target might include potentially unsafe "..". ++#if defined(SAVE_LINKS) && defined(_UNIX) || defined(_WIN_ALL) ++ if (Arc.Format==RARFMT50) // For RAR5 archives we can check RedirName for both Unix and Windows. ++ UpLink=wcsstr(Arc.FileHead.RedirName,L"..")!=NULL; ++#endif ++ + #if defined(SAVE_LINKS) && defined(_UNIX) + // For RAR 3.x archives we process links even in test mode to skip link data. + if (Arc.Format==RARFMT15) +- return ExtractUnixLink30(Cmd,DataIO,Arc,LinkName); ++ return ExtractUnixLink30(Cmd,DataIO,Arc,LinkName,UpLink); + if (Arc.Format==RARFMT50) + return ExtractUnixLink50(Cmd,LinkName,&Arc.FileHead); +-#elif defined _WIN_ALL ++#elif defined(_WIN_ALL) + // RAR 5.0 archives store link information in file header, so there is + // no need to additionally test it if we do not create a file. + if (Arc.Format==RARFMT50) +diff --git a/extinfo.hpp b/extinfo.hpp +index 2b0005d..9c42f7d 100644 +--- a/extinfo.hpp ++++ b/extinfo.hpp +@@ -1,8 +1,9 @@ + #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 ExtractSymlink(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const wchar *LinkName,bool &UpLink); + #ifdef _UNIX + void SetUnixOwner(Archive &Arc,const wchar *FileName); + #endif +diff --git a/extract.cpp b/extract.cpp +index abcd3c3..21c9c4a 100644 +--- a/extract.cpp ++++ b/extract.cpp +@@ -9,6 +9,12 @@ CmdExtract::CmdExtract(CommandData *Cmd) + *DestFileName=0; + + 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. ++ UpLinkExtracted=false; ++ + Unp=new Unpack(&DataIO); + #ifdef RAR_SMP + Unp->SetThreads(Cmd->Threads); +@@ -99,6 +105,8 @@ void CmdExtract::ExtractArchiveInit(Archive &Arc) + AnySolidDataUnpackedWell=false; + + StartTime.SetCurrentTime(); ++ ++ LastCheckedSymlink.clear(); + } + + +@@ -523,6 +531,10 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) + wcsncpyz(DestFileName,Cmd->DllDestName,ASIZE(DestFileName)); + #endif + ++ if (ExtrFile && Command!='P' && !Cmd->Test && !Cmd->AbsoluteLinks && ++ UpLinkExtracted) ++ ExtrFile=LinksToDirs(DestFileName,Cmd->ExtrPath,LastCheckedSymlink); ++ + File CurFile; + + bool LinkEntry=Arc.FileHead.RedirType!=FSREDIR_NONE; +@@ -662,7 +674,22 @@ bool CmdExtract::ExtractCurrentFile(Archive &Arc,size_t HeaderSize,bool &Repeat) + if (Type==FSREDIR_UNIXSYMLINK || Type==FSREDIR_WINSYMLINK || Type==FSREDIR_JUNCTION) + { + if (FileCreateMode) +- LinkSuccess=ExtractSymlink(Cmd,DataIO,Arc,DestFileName); ++ { ++ bool UpLink; ++ LinkSuccess=ExtractSymlink(Cmd,DataIO,Arc,DestFileName,UpLink); ++ UpLinkExtracted|=LinkSuccess && UpLink; ++ ++ // We do not actually need to reset the cache here if we cache ++ // only the single last checked path, because at this point ++ // it will always contain the link own path and link can't ++ // overwrite its parent folder. But if we ever decide to cache ++ // several already checked paths, we'll need to reset them here. ++ // Otherwise if no files were created in one of such paths, ++ // let's say because of file create error, it might be possible ++ // to overwrite the path with link and avoid checks. We keep this ++ // code here as a reminder in case of possible modifications. ++ LastCheckedSymlink.clear(); // Reset cache for safety reason. ++ } + } + else + { +@@ -843,8 +870,6 @@ void CmdExtract::UnstoreFile(ComprDataIO &DataIO,int64 DestUnpSize) + + bool CmdExtract::ExtractFileCopy(File &New,wchar *ArcName,wchar *NameNew,wchar *NameExisting,size_t NameExistingSize) + { +- SlashToNative(NameExisting,NameExisting,NameExistingSize); // Not needed for RAR 5.1+ archives. +- + File Existing; + if (!Existing.WOpen(NameExisting)) + { +@@ -1107,8 +1132,15 @@ void CmdExtract::ExtrCreateDir(Archive &Arc,const wchar *ArcFileName) + wchar OrigName[ASIZE(DestFileName)]; + wcsncpyz(OrigName,DestFileName,ASIZE(OrigName)); + MakeNameUsable(DestFileName,true); +- CreatePath(DestFileName,true); +- MDCode=MakeDir(DestFileName,!Cmd->IgnoreGeneralAttr,Arc.FileHead.FileAttr); ++ DirExist=FileExist(DestFileName) && IsDir(GetFileAttr(DestFileName)); ++ if (!DirExist) ++ { ++ if (!Cmd->AbsoluteLinks && UpLinkExtracted) ++ LinksToDirs(DestFileName,Cmd->ExtrPath,LastCheckedSymlink); ++ ++ CreatePath(DestFileName,true); ++ MDCode=MakeDir(DestFileName,!Cmd->IgnoreGeneralAttr,Arc.FileHead.FileAttr); ++ } + #ifndef SFX_MODULE + if (MDCode==MKDIR_SUCCESS) + uiMsg(UIERROR_RENAMING,Arc.FileName,OrigName,DestFileName); +@@ -1191,6 +1223,8 @@ bool CmdExtract::ExtrCreateFile(Archive &Arc,File &CurFile) + + MakeNameUsable(DestFileName,true); + ++ if (!Cmd->AbsoluteLinks && UpLinkExtracted) ++ LinksToDirs(DestFileName,Cmd->ExtrPath,LastCheckedSymlink); + CreatePath(DestFileName,true); + if (FileCreate(Cmd,&CurFile,DestFileName,ASIZE(DestFileName),&UserReject,Arc.FileHead.UnpSize,&Arc.FileHead.mtime,true)) + { +diff --git a/extract.hpp b/extract.hpp +index 159759b..eadb8cf 100644 +--- a/extract.hpp ++++ b/extract.hpp +@@ -52,6 +52,12 @@ class CmdExtract + bool PrevProcessed; // If previous file was successfully extracted or tested. + wchar DestFileName[NM]; + bool PasswordCancelled; ++ bool UpLinkExtracted; // At least one symlink with ".." in target was extracted. ++ ++ // Last path checked for symlinks. We use it to improve the performance, ++ // so we do not check recently checked folders again. ++ std::wstring LastCheckedSymlink; ++ + #if defined(_WIN_ALL) && !defined(SFX_MODULE) && !defined(SILENT) + bool Fat32,NotFat32; + #endif +diff --git a/hardlinks.cpp b/hardlinks.cpp +index 946a395..4b980de 100644 +--- a/hardlinks.cpp ++++ b/hardlinks.cpp +@@ -1,7 +1,5 @@ + bool ExtractHardlink(wchar *NameNew,wchar *NameExisting,size_t NameExistingSize) + { +- SlashToNative(NameExisting,NameExisting,NameExistingSize); // Not needed for RAR 5.1+ archives. +- + if (!FileExist(NameExisting)) + { + uiMsg(UIERROR_HLINKCREATE,NameNew); +diff --git a/model.cpp b/model.cpp +index 83391c5..e4f9e3c 100644 +--- a/model.cpp ++++ b/model.cpp +@@ -532,13 +532,15 @@ inline bool RARPPM_CONTEXT::decodeSymbol2(ModelPPM *Model) + Model->Coder.SubRange.LowCount=HiCnt; + Model->Coder.SubRange.HighCount=Model->Coder.SubRange.scale; + i=NumStats-Model->NumMasked; +- pps--; ++ ++ // 2022.12.02: we removed pps-- here and changed the code below to avoid ++ // "array subscript -1 is outside array bounds" warning in some compilers. + do + { +- pps++; + if (pps>=ps+ASIZE(ps)) // Extra safety check. + return false; + Model->CharMask[(*pps)->Symbol]=Model->EscCount; ++ pps++; + } while ( --i ); + psee2c->Summ += Model->Coder.SubRange.scale; + Model->NumMasked = NumStats; +diff --git a/os.hpp b/os.hpp +index b69f348..a73dc7c 100644 +--- a/os.hpp ++++ b/os.hpp +@@ -13,6 +13,7 @@ + #endif + + #include <new> ++#include <string> + + + #if defined(_WIN_ALL) || defined(_EMX) +diff --git a/pathfn.cpp b/pathfn.cpp +index 41594bf..d454052 100644 +--- a/pathfn.cpp ++++ b/pathfn.cpp +@@ -31,11 +31,17 @@ wchar* ConvertPath(const wchar *SrcPath,wchar *DestPath,size_t DestSize) + const wchar *s=DestPtr; + if (s[0]!=0 && IsDriveDiv(s[1])) + s+=2; +- if (s[0]=='\\' && s[1]=='\\') ++ ++ // Skip UNC Windows \\server\share\ or Unix //server/share/ ++ if (IsPathDiv(s[0]) && IsPathDiv(s[1])) + { +- const wchar *Slash=wcschr(s+2,'\\'); +- if (Slash!=NULL && (Slash=wcschr(Slash+1,'\\'))!=NULL) +- s=Slash+1; ++ uint SlashCount=0; ++ for (const wchar *t=s+2;*t!=0;t++) ++ if (IsPathDiv(*t) && ++SlashCount==2) ++ { ++ s=t+1; // Found two more path separators after leading two. ++ break; ++ } + } + for (const wchar *t=s;*t!=0;t++) + if (IsPathDiv(*t)) +diff --git a/timefn.hpp b/timefn.hpp +index 5271361..49b61e8 100644 +--- a/timefn.hpp ++++ b/timefn.hpp +@@ -22,6 +22,17 @@ class RarTime + + // Internal time representation in 1/TICKS_PER_SECOND since 01.01.1601. + // We use nanoseconds here to handle the high precision Unix time. ++ // It allows dates up to July 2185. ++ // ++ // If we'll ever need to extend the date range, we can define a lower ++ // precision Windows version of TICKS_PER_SECOND. But then Unix and Windows ++ // versions can differ in least significant digits of "lt" time output ++ // for Unix archives. ++ // Alternatively we can introduce 'bool HighPrecision' set to true ++ // in SetUnixNS() and TicksPerSecond() instead of constant above. ++ // It might be more reliable than defining TicksPerSecond variable, ++ // which wouldn't survive memset of any structure hosting RarTime. ++ // We would need to eliminate all such memsets in the entire code first. + uint64 itime; + public: + // RarLocalTime::Reminder precision. Must be equal to TICKS_PER_SECOND. +diff --git a/ulinks.cpp b/ulinks.cpp +index 2555ac4..9c1cb7c 100644 +--- a/ulinks.cpp ++++ b/ulinks.cpp +@@ -65,7 +65,8 @@ static bool SafeCharToWide(const char *Src,wchar *Dest,size_t DestSize) + } + + +-bool ExtractUnixLink30(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const wchar *LinkName) ++static bool ExtractUnixLink30(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc, ++ const wchar *LinkName,bool &UpLink) + { + char Target[NM]; + if (IsLink(Arc.FileHead.FileAttr)) +@@ -95,13 +96,14 @@ bool ExtractUnixLink30(CommandData *Cmd,ComprDataIO &DataIO,Archive &Arc,const w + if (!Cmd->AbsoluteLinks && (IsFullPath(TargetW) || + !IsRelativeSymlinkSafe(Cmd,Arc.FileHead.FileName,LinkName,TargetW))) + return false; ++ UpLink=strstr(Target,"..")!=NULL; + return UnixSymlink(Target,LinkName,&Arc.FileHead.mtime,&Arc.FileHead.atime); + } + return false; + } + + +-bool ExtractUnixLink50(CommandData *Cmd,const wchar *Name,FileHeader *hd) ++static bool ExtractUnixLink50(CommandData *Cmd,const wchar *Name,FileHeader *hd) + { + char Target[NM]; + WideToChar(hd->RedirName,Target,ASIZE(Target)); +diff --git a/win32stm.cpp b/win32stm.cpp +index eaa43be..3a1e384 100644 +--- a/win32stm.cpp ++++ b/win32stm.cpp +@@ -117,8 +117,13 @@ void ExtractStreams(Archive &Arc,const wchar *FileName,bool TestMode) + if ((fd.FileAttr & FILE_ATTRIBUTE_READONLY)!=0) + SetFileAttr(FileName,fd.FileAttr & ~FILE_ATTRIBUTE_READONLY); + File CurFile; +- if (CurFile.WCreate(FullName) && Arc.ReadSubData(NULL,&CurFile,false)) +- CurFile.Close(); ++ ++ if (CurFile.WCreate(FullName)) ++ { ++ if (Arc.ReadSubData(NULL,&CurFile)) ++ CurFile.Close(); ++ } ++ + File HostFile; + if (Found && HostFile.Open(FileName,FMF_OPENSHARED|FMF_UPDATE)) + SetFileTime(HostFile.GetHandle(),&fd.ftCreationTime,&fd.ftLastAccessTime, diff -Nru unrar-nonfree-6.0.3/debian/patches/series unrar-nonfree-6.0.3/debian/patches/series --- unrar-nonfree-6.0.3/debian/patches/series 2022-05-10 13:26:16.000000000 +0200 +++ unrar-nonfree-6.0.3/debian/patches/series 2023-08-20 09:58:26.000000000 +0200 @@ -10,3 +10,4 @@ 0010-Display-build-commands.patch 0011-Add-visibility-attribute-to-reduce-export-table.patch 0012-Fix-CVE-2022-30333.patch +0013-CVE-2022-48579.patch
