Control: tags -1 patch Hi!
On Fri, 2014-03-28 at 14:42:41 +0100, Guillem Jover wrote: > Package: apt > Version: 0.9.16.1 > Severity: normal > Somewhat recently apt was fixed to add LFS for the ar containers, but > the tarballs within are still not LFS-safe on 32-bit systems. > > Here's a list of issues I've spotted by code staring, I've not tested > anything, and I should create LFS .deb tests for the tar members too > in dpkg/pkg-tests.git. > > Types (should be off_t, long long or any other 64-bit-safe type): > > - ARArchive::Member::Start. > - pkgDirStream::Size. > - pkgDirStream::Process(), Size and Pos arguments. > - ExtractTar::Go(), Size and Read variables, and cast truncation. > > The following I guess more out of correctness, as I don't expect to > see > 4 GiB control files around: > > - debDebFile::MemControlExtract::Length. > - debDebFile::MemControlExtract::Process(), Size and Pos arguments. > - debDebFile::MemControlExtract::TakeControl(), Size argument. > > These are minor issues, and would be related to either bogus or > malicious archives, but probably still good to handle: > > - ExtractTar::Go(), GNU_LongLink and GNU_LongName short Length which > would truncate from Itm.Size. Ok, here's a first rough go at a patch. It breaks ABI, and just noticed an ABI breaking release was recently uploaded to experimental. :( Just wanted to publish it for now, in case your policy allows to merge this in the ABI breaking release. Otherwise I could rework it to stage the change in preprocessor macros in a similar way as how you seem to handle these. I've only test-built it though. Some other comments: * MaxInSize is not used anywhere, but I guess it could be useful to handle bundled archives. Otherwise it might make sense to just remove it, and the arg in the constructor. * Also changed the in-core loaders because they are virtuals. * The in-core handling could possibly check for huge sizes before trying to allocate stuff, but it should fail with ENOMEM or std::bad_alloc otherwise I guess. * The patch description is rather terseā¦ Thanks, Guillem
From 78a9cbf46aa0d073b8f88d949df5495f73a4fa23 Mon Sep 17 00:00:00 2001 From: Guillem Jover <guil...@debian.org> Date: Wed, 2 Jul 2014 03:12:00 +0200 Subject: [PATCH 1/2] Add new Base256ToNum long long overload function --- apt-pkg/contrib/strutl.cc | 19 ++++++++++++++++++- apt-pkg/contrib/strutl.h | 1 + debian/libapt-pkg4.12.symbols | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/apt-pkg/contrib/strutl.cc b/apt-pkg/contrib/strutl.cc index ce69c7a..0f48860 100644 --- a/apt-pkg/contrib/strutl.cc +++ b/apt-pkg/contrib/strutl.cc @@ -1046,7 +1046,7 @@ bool StrToNum(const char *Str,unsigned long long &Res,unsigned Len,unsigned Base // --------------------------------------------------------------------- /* This is used in decoding the 256bit encoded fixed length fields in tar files */ -bool Base256ToNum(const char *Str,unsigned long &Res,unsigned int Len) +bool Base256ToNum(const char *Str,unsigned long long &Res,unsigned int Len) { if ((Str[0] & 0x80) == 0) return false; @@ -1059,6 +1059,23 @@ bool Base256ToNum(const char *Str,unsigned long &Res,unsigned int Len) } } /*}}}*/ +// Base256ToNum - Convert a fixed length binary to a number /*{{{*/ +// --------------------------------------------------------------------- +/* This is used in decoding the 256bit encoded fixed length fields in + tar files */ +bool Base256ToNum(const char *Str,unsigned long &Res,unsigned int Len) +{ + unsigned long long Num; + bool rc; + + rc = Base256ToNum(Str, Num, Len); + Res = Num; + if (Res != Num) + return false; + + return rc; +} + /*}}}*/ // HexDigit - Convert a hex character into an integer /*{{{*/ // --------------------------------------------------------------------- /* Helper for Hex2Num */ diff --git a/apt-pkg/contrib/strutl.h b/apt-pkg/contrib/strutl.h index 185cdc3..5733fd6 100644 --- a/apt-pkg/contrib/strutl.h +++ b/apt-pkg/contrib/strutl.h @@ -72,6 +72,7 @@ bool ReadMessages(int Fd, std::vector<std::string> &List); bool StrToNum(const char *Str,unsigned long &Res,unsigned Len,unsigned Base = 0); bool StrToNum(const char *Str,unsigned long long &Res,unsigned Len,unsigned Base = 0); bool Base256ToNum(const char *Str,unsigned long &Res,unsigned int Len); +bool Base256ToNum(const char *Str,unsigned long long &Res,unsigned int Len); bool Hex2Num(const std::string &Str,unsigned char *Num,unsigned int Length); // input changing string split diff --git a/debian/libapt-pkg4.12.symbols b/debian/libapt-pkg4.12.symbols index 3fa128c..f99d3dd 100644 --- a/debian/libapt-pkg4.12.symbols +++ b/debian/libapt-pkg4.12.symbols @@ -22,6 +22,7 @@ libapt-pkg.so.4.12 libapt-pkg4.12 #MINVER# (c++)"StringToBool(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, int)@Base" 0.8.0 (c++)"UnmountCdrom(std::basic_string<char, std::char_traits<char>, std::allocator<char> >)@Base" 0.8.0 (c++)"_GetErrorObj()@Base" 0.8.0 + (c++)"Base256ToNum(char const*, unsigned long long&, unsigned int)@Base" 1.0.5 (c++)"pkgFixBroken(pkgDepCache&)@Base" 0.8.0 (c++)"DeQuoteString(__gnu_cxx::__normal_iterator<char const*, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > const&, __gnu_cxx::__normal_iterator<char const*, std::basic_string<char, std::char_traits<char>, std::allocator<char> > > const&)@Base" 0.8.0 (c++)"DeQuoteString(std::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)@Base" 0.8.0 -- 2.0.1
From 6f85a5134282862fc87126cb57ed6ae1becdf6fc Mon Sep 17 00:00:00 2001 From: Guillem Jover <guil...@debian.org> Date: Wed, 2 Jul 2014 03:10:21 +0200 Subject: [PATCH 2/2] Fix ar and tar code to be LFS-safe This is an ABI break. Closes: #742882 --- apt-inst/contrib/arfile.h | 2 +- apt-inst/contrib/extracttar.cc | 13 ++++++------- apt-inst/contrib/extracttar.h | 4 ++-- apt-inst/deb/debfile.cc | 4 ++-- apt-inst/deb/debfile.h | 4 ++-- apt-inst/dirstream.h | 4 ++-- cmdline/apt-extracttemplates.cc | 2 +- cmdline/apt-extracttemplates.h | 4 ++-- debian/libapt-inst1.5.symbols | 8 ++++---- 9 files changed, 22 insertions(+), 23 deletions(-) diff --git a/apt-inst/contrib/arfile.h b/apt-inst/contrib/arfile.h index 0f62a34..5aa38ae 100644 --- a/apt-inst/contrib/arfile.h +++ b/apt-inst/contrib/arfile.h @@ -61,7 +61,7 @@ struct ARArchive::Member unsigned long long Size; // Location of the data. - unsigned long Start; + unsigned long long Start; Member *Next; Member() : Start(0), Next(0) {}; diff --git a/apt-inst/contrib/extracttar.cc b/apt-inst/contrib/extracttar.cc index 0ba3f05..2c86d0d 100644 --- a/apt-inst/contrib/extracttar.cc +++ b/apt-inst/contrib/extracttar.cc @@ -60,9 +60,8 @@ struct ExtractTar::TarHeader // ExtractTar::ExtractTar - Constructor /*{{{*/ // --------------------------------------------------------------------- /* */ -ExtractTar::ExtractTar(FileFd &Fd,unsigned long Max,string DecompressionProgram) : File(Fd), - MaxInSize(Max), DecompressProg(DecompressionProgram) - +ExtractTar::ExtractTar(FileFd &Fd,unsigned long long Max,string DecompressionProgram) + : File(Fd), MaxInSize(Max), DecompressProg(DecompressionProgram) { GZPid = -1; Eof = false; @@ -267,7 +266,7 @@ bool ExtractTar::Go(pkgDirStream &Stream) case GNU_LongLink: { - unsigned long Length = Itm.Size; + unsigned long long Length = Itm.Size; unsigned char Block[512]; while (Length > 0) { @@ -286,7 +285,7 @@ bool ExtractTar::Go(pkgDirStream &Stream) case GNU_LongName: { - unsigned long Length = Itm.Size; + unsigned long long Length = Itm.Size; unsigned char Block[512]; while (Length > 0) { @@ -315,11 +314,11 @@ bool ExtractTar::Go(pkgDirStream &Stream) return false; // Copy the file over the FD - unsigned long Size = Itm.Size; + unsigned long long Size = Itm.Size; while (Size != 0) { unsigned char Junk[32*1024]; - unsigned long Read = min(Size,(unsigned long)sizeof(Junk)); + unsigned long Read = min(Size, (unsigned long long)sizeof(Junk)); if (InFd.Read(Junk,((Read+511)/512)*512) == false) return false; diff --git a/apt-inst/contrib/extracttar.h b/apt-inst/contrib/extracttar.h index 4b29df3..472e018 100644 --- a/apt-inst/contrib/extracttar.h +++ b/apt-inst/contrib/extracttar.h @@ -39,7 +39,7 @@ class ExtractTar GNU_LongLink = 'K',GNU_LongName = 'L'}; FileFd &File; - unsigned long MaxInSize; + unsigned long long MaxInSize; int GZPid; FileFd InFd; bool Eof; @@ -53,7 +53,7 @@ class ExtractTar bool Go(pkgDirStream &Stream); - ExtractTar(FileFd &Fd,unsigned long Max,std::string DecompressionProgram); + ExtractTar(FileFd &Fd,unsigned long long Max,std::string DecompressionProgram); virtual ~ExtractTar(); }; diff --git a/apt-inst/deb/debfile.cc b/apt-inst/deb/debfile.cc index a63cb67..4853a13 100644 --- a/apt-inst/deb/debfile.cc +++ b/apt-inst/deb/debfile.cc @@ -203,7 +203,7 @@ bool debDebFile::MemControlExtract::DoItem(Item &Itm,int &Fd) /* Just memcopy the block from the tar extractor and put it in the right place in the pre-allocated memory block. */ bool debDebFile::MemControlExtract::Process(Item &/*Itm*/,const unsigned char *Data, - unsigned long Size,unsigned long Pos) + unsigned long long Size,unsigned long long Pos) { memcpy(Control + Pos, Data,Size); return true; @@ -232,7 +232,7 @@ bool debDebFile::MemControlExtract::Read(debDebFile &Deb) // --------------------------------------------------------------------- /* The given memory block is loaded into the parser and parsed as a control record. */ -bool debDebFile::MemControlExtract::TakeControl(const void *Data,unsigned long Size) +bool debDebFile::MemControlExtract::TakeControl(const void *Data,unsigned long long Size) { delete [] Control; Control = new char[Size+2]; diff --git a/apt-inst/deb/debfile.h b/apt-inst/deb/debfile.h index 880bcf6..b068efc 100644 --- a/apt-inst/deb/debfile.h +++ b/apt-inst/deb/debfile.h @@ -81,12 +81,12 @@ class debDebFile::MemControlExtract : public pkgDirStream // Members from DirStream virtual bool DoItem(Item &Itm,int &Fd); virtual bool Process(Item &Itm,const unsigned char *Data, - unsigned long Size,unsigned long Pos); + unsigned long long Size,unsigned long long Pos); // Helpers bool Read(debDebFile &Deb); - bool TakeControl(const void *Data,unsigned long Size); + bool TakeControl(const void *Data,unsigned long long Size); MemControlExtract() : IsControl(false), Control(0), Length(0), Member("control") {}; MemControlExtract(std::string Member) : IsControl(false), Control(0), Length(0), Member(Member) {}; diff --git a/apt-inst/dirstream.h b/apt-inst/dirstream.h index 1be2688..571fe86 100644 --- a/apt-inst/dirstream.h +++ b/apt-inst/dirstream.h @@ -37,10 +37,10 @@ class pkgDirStream Directory, FIFO} Type; char *Name; char *LinkTarget; + unsigned long long Size; unsigned long Mode; unsigned long UID; unsigned long GID; - unsigned long Size; unsigned long MTime; unsigned long Major; unsigned long Minor; @@ -50,7 +50,7 @@ class pkgDirStream virtual bool Fail(Item &Itm,int Fd); virtual bool FinishedFile(Item &Itm,int Fd); virtual bool Process(Item &/*Itm*/,const unsigned char * /*Data*/, - unsigned long /*Size*/,unsigned long /*Pos*/) {return true;}; + unsigned long long /*Size*/,unsigned long long /*Pos*/) {return true;}; virtual ~pkgDirStream() {}; }; diff --git a/cmdline/apt-extracttemplates.cc b/cmdline/apt-extracttemplates.cc index e4428e0..e2d0c0d 100644 --- a/cmdline/apt-extracttemplates.cc +++ b/cmdline/apt-extracttemplates.cc @@ -138,7 +138,7 @@ bool DebFile::DoItem(Item &I, int &Fd) // --------------------------------------------------------------------- /* */ bool DebFile::Process(Item &/*I*/, const unsigned char *data, - unsigned long size, unsigned long pos) + unsigned long long size, unsigned long long pos) { switch (Which) { diff --git a/cmdline/apt-extracttemplates.h b/cmdline/apt-extracttemplates.h index 9cc3f5f..27137df 100644 --- a/cmdline/apt-extracttemplates.h +++ b/cmdline/apt-extracttemplates.h @@ -20,7 +20,7 @@ class pkgCache; class DebFile : public pkgDirStream { FileFd File; - unsigned long Size; + unsigned long long Size; char *Control; unsigned long ControlLen; @@ -29,7 +29,7 @@ public: ~DebFile(); bool DoItem(Item &I, int &fd); bool Process(pkgDirStream::Item &I, const unsigned char *data, - unsigned long size, unsigned long pos); + unsigned long long size, unsigned long long pos); bool Go(); bool ParseInfo(); diff --git a/debian/libapt-inst1.5.symbols b/debian/libapt-inst1.5.symbols index 8ce7072..c5ab7f4 100644 --- a/debian/libapt-inst1.5.symbols +++ b/debian/libapt-inst1.5.symbols @@ -3,7 +3,7 @@ libapt-inst.so.1.5 libapt-inst1.5 #MINVER# (c++)"ExtractTar::Done(bool)@Base" 0.8.0 (c++)"ExtractTar::Go(pkgDirStream&)@Base" 0.8.0 (c++)"ExtractTar::StartGzip()@Base" 0.8.0 - (c++)"ExtractTar::ExtractTar(FileFd&, unsigned long, std::basic_string<char, std::char_traits<char>, std::allocator<char> >)@Base" 0.8.0 + (c++)"ExtractTar::ExtractTar(FileFd&, unsigned long long, std::basic_string<char, std::char_traits<char>, std::allocator<char> >)@Base" 1.0.5 (c++)"ExtractTar::~ExtractTar()@Base" 0.8.0 (c++)"debDebFile::GotoMember(char const*)@Base" 0.8.0 (c++)"debDebFile::CheckMember(char const*)@Base" 0.8.0 @@ -11,10 +11,10 @@ libapt-inst.so.1.5 libapt-inst1.5 #MINVER# (c++)"debDebFile::ControlExtract::~ControlExtract()@Base" 0.8.0 (c++)"debDebFile::ExtractTarMember(pkgDirStream&, char const*)@Base" 0.9.15.4 (c++)"debDebFile::ExtractArchive(pkgDirStream&)@Base" 0.8.0 - (c++)"debDebFile::MemControlExtract::TakeControl(void const*, unsigned long)@Base" 0.8.0 + (c++)"debDebFile::MemControlExtract::TakeControl(void const*, unsigned long long)@Base" 1.0.5 (c++)"debDebFile::MemControlExtract::Read(debDebFile&)@Base" 0.8.0 (c++)"debDebFile::MemControlExtract::DoItem(pkgDirStream::Item&, int&)@Base" 0.8.0 - (c++)"debDebFile::MemControlExtract::Process(pkgDirStream::Item&, unsigned char const*, unsigned long, unsigned long)@Base" 0.8.0 + (c++)"debDebFile::MemControlExtract::Process(pkgDirStream::Item&, unsigned char const*, unsigned long long, unsigned long long)@Base" 1.0.5 (c++)"debDebFile::MemControlExtract::~MemControlExtract()@Base" 0.8.0 (c++)"debDebFile::debDebFile(FileFd&)@Base" 0.8.0 (c++)"pkgExtract::FinishedFile(pkgDirStream::Item&, int)@Base" 0.8.0 @@ -41,7 +41,7 @@ libapt-inst.so.1.5 libapt-inst1.5 #MINVER# (c++)"pkgDirStream::FinishedFile(pkgDirStream::Item&, int)@Base" 0.8.0 (c++)"pkgDirStream::Fail(pkgDirStream::Item&, int)@Base" 0.8.0 (c++)"pkgDirStream::DoItem(pkgDirStream::Item&, int&)@Base" 0.8.0 - (c++)"pkgDirStream::Process(pkgDirStream::Item&, unsigned char const*, unsigned long, unsigned long)@Base" 0.8.0 + (c++)"pkgDirStream::Process(pkgDirStream::Item&, unsigned char const*, unsigned long long, unsigned long long)@Base" 1.0.5 (c++)"pkgDirStream::~pkgDirStream()@Base" 0.8.0 (c++|optional)"pkgCache::DepIterator::operator++(int)@Base" 0.8.0 (c++|optional)"pkgCache::DepIterator::operator++()@Base" 0.8.0 -- 2.0.1