--- Begin Message ---
- To: Debian Bug Tracking System <submit@bugs.debian.org>
- Subject: bullseye-pu: package unrar-nonfree/1:6.0.3-1+deb11u1
- From: Markus Koschany <apo@debian.org>
- Date: Sun, 20 Aug 2023 10:15:53 +0200
- Message-id: <169251935336.9153.1328939860305142052.reportbug@faye>
Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: apo@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 <apo@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 <apo@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
--- End Message ---