Browse Source

try not to call memcpy with length 0 in hash calculations

memcpy is marked as nonnull for its input, but ignores the input anyhow
if the declared length is zero. Our SHA2 implementations do this as
well, it was "just" MD5 and SHA1 missing, so we add the length check
here as well as along the callstack as it is really pointless to do all
these method calls for "nothing".

Reported-By: gcc -fsanitize=undefined
David Kalnischkies 7 years ago
parent
commit
644478e8db

+ 2 - 0
apt-pkg/contrib/hashes.cc

@@ -312,6 +312,8 @@ public:
 // Hashes::Add* - Add the contents of data or FD			/*{{{*/
 bool Hashes::Add(const unsigned char * const Data, unsigned long long const Size)
 {
+   if (Size == 0)
+      return true;
    bool Res = true;
 APT_IGNORE_DEPRECATED_PUSH
    if ((d->CalcHashes & MD5SUM) == MD5SUM)

+ 4 - 4
apt-pkg/contrib/hashes.h

@@ -195,11 +195,11 @@ class Hashes
 
    static const int UntilEOF = 0;
 
-   bool Add(const unsigned char * const Data, unsigned long long const Size);
-   APT_DEPRECATED_MSG("Construct accordingly instead of choosing hashes while adding") bool Add(const unsigned char * const Data, unsigned long long const Size, unsigned int const Hashes);
-   inline bool Add(const char * const Data)
+   bool Add(const unsigned char * const Data, unsigned long long const Size) APT_NONNULL(2);
+   APT_DEPRECATED_MSG("Construct accordingly instead of choosing hashes while adding") bool Add(const unsigned char * const Data, unsigned long long const Size, unsigned int const Hashes) APT_NONNULL(2);
+   inline bool Add(const char * const Data) APT_NONNULL(2)
    {return Add((unsigned char const * const)Data,strlen(Data));};
-   inline bool Add(const unsigned char * const Beg,const unsigned char * const End)
+   inline bool Add(const unsigned char * const Beg,const unsigned char * const End) APT_NONNULL(2,3)
    {return Add(Beg,End-Beg);};
 
    enum SupportedHashes { MD5SUM = (1 << 0), SHA1SUM = (1 << 1), SHA256SUM = (1 << 2),

+ 6 - 6
apt-pkg/contrib/hashsum_template.h

@@ -122,18 +122,18 @@ class HashSumValue
 class SummationImplementation
 {
    public:
-   virtual bool Add(const unsigned char *inbuf, unsigned long long inlen) = 0;
-   inline bool Add(const char *inbuf, unsigned long long const inlen)
+   virtual bool Add(const unsigned char *inbuf, unsigned long long inlen) APT_NONNULL(2) = 0;
+   inline bool Add(const char *inbuf, unsigned long long const inlen) APT_NONNULL(2)
    { return Add((const unsigned char *)inbuf, inlen); }
 
-   inline bool Add(const unsigned char *Data)
+   inline bool Add(const unsigned char *Data) APT_NONNULL(2)
    { return Add(Data, strlen((const char *)Data)); }
-   inline bool Add(const char *Data)
+   inline bool Add(const char *Data) APT_NONNULL(2)
    { return Add((const unsigned char *)Data, strlen(Data)); }
 
-   inline bool Add(const unsigned char *Beg, const unsigned char *End)
+   inline bool Add(const unsigned char *Beg, const unsigned char *End) APT_NONNULL(2,3)
    { return Add(Beg, End - Beg); }
-   inline bool Add(const char *Beg, const char *End)
+   inline bool Add(const char *Beg, const char *End) APT_NONNULL(2,3)
    { return Add((const unsigned char *)Beg, End - Beg); }
 
    bool AddFD(int Fd, unsigned long long Size = 0);

+ 2 - 0
apt-pkg/contrib/md5.cc

@@ -187,6 +187,8 @@ bool MD5Summation::Add(const unsigned char *data,unsigned long long len)
 {
    if (Done == true)
       return false;
+   if (len == 0)
+      return true;
 
    uint32_t *buf = (uint32_t *)Buf;
    uint32_t *bytes = (uint32_t *)Bytes;

+ 1 - 1
apt-pkg/contrib/md5.h

@@ -48,7 +48,7 @@ class MD5Summation : public SummationImplementation
 
    public:
 
-   bool Add(const unsigned char *inbuf, unsigned long long inlen) APT_OVERRIDE;
+   bool Add(const unsigned char *inbuf, unsigned long long inlen) APT_OVERRIDE APT_NONNULL(2);
    using SummationImplementation::Add;
 
    MD5SumValue Result();

+ 2 - 0
apt-pkg/contrib/sha1.cc

@@ -243,6 +243,8 @@ bool SHA1Summation::Add(const unsigned char *data,unsigned long long len)
 {
    if (Done)
       return false;
+   if (len == 0)
+      return true;
 
    uint32_t *state = (uint32_t *)State;
    uint32_t *count = (uint32_t *)Count;

+ 1 - 1
apt-pkg/contrib/sha1.h

@@ -37,7 +37,7 @@ class SHA1Summation : public SummationImplementation
    bool Done;
    
    public:
-   bool Add(const unsigned char *inbuf, unsigned long long inlen) APT_OVERRIDE;
+   bool Add(const unsigned char *inbuf, unsigned long long inlen) APT_OVERRIDE APT_NONNULL(2);
    using SummationImplementation::Add;
 
    SHA1SumValue Result();

+ 3 - 3
apt-pkg/contrib/sha2.h

@@ -34,7 +34,7 @@ class SHA2SummationBase : public SummationImplementation
  protected:
    bool Done;
  public:
-   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE = 0;
+   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE APT_NONNULL(2) = 0;
 
    void Result();
 };
@@ -45,7 +45,7 @@ class SHA256Summation : public SHA2SummationBase
    unsigned char Sum[32];
 
    public:
-   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE
+   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE APT_NONNULL(2)
    {
       if (Done) 
          return false;
@@ -78,7 +78,7 @@ class SHA512Summation : public SHA2SummationBase
    unsigned char Sum[64];
 
    public:
-   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE
+   bool Add(const unsigned char *inbuf, unsigned long long len) APT_OVERRIDE APT_NONNULL(2)
    {
       if (Done) 
          return false;

+ 5 - 4
methods/rred.cc

@@ -335,7 +335,7 @@ class Patch {
    FileChanges filechanges;
    MemBlock add_text;
 
-   static bool retry_fwrite(char *b, size_t l, FileFd &f, Hashes * const start_hash, Hashes * const end_hash = nullptr)
+   static bool retry_fwrite(char *b, size_t l, FileFd &f, Hashes * const start_hash, Hashes * const end_hash = nullptr) APT_NONNULL(1)
    {
       if (f.Write(b, l) == false)
 	 return false;
@@ -385,8 +385,8 @@ class Patch {
       }
    }
 
-   static void dump_mem(FileFd &o, char *p, size_t s, Hashes *hash) {
-      retry_fwrite(p, s, o, hash);
+   static void dump_mem(FileFd &o, char *p, size_t s, Hashes *hash) APT_NONNULL(2) {
+      retry_fwrite(p, s, o, nullptr, hash);
    }
 
    public:
@@ -538,7 +538,8 @@ class Patch {
       for (ch = filechanges.begin(); ch != filechanges.end(); ++ch) {
 	 dump_lines(out, in, ch->offset, start_hash, end_hash);
 	 skip_lines(in, ch->del_cnt, start_hash);
-	 dump_mem(out, ch->add, ch->add_len, end_hash);
+	 if (ch->add_len != 0)
+	    dump_mem(out, ch->add, ch->add_len, end_hash);
       }
       dump_rest(out, in, start_hash, end_hash);
       out.Flush();