From: Amos Jeffries <squid3@treenet.co.nz>
Date: Thu, 29 Oct 2015 18:53:48 +0000 (-0700)
Subject: Add Locker friend class to SBuf for protection against memory issues
X-Git-Tag: SQUID_4_0_2~4^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4a0fcd9b61eb72639e3f72dc4b3a0b4b977ff995;p=thirdparty%2Fsquid.git

Add Locker friend class to SBuf for protection against memory issues

When appending or otherwise modifying an SBuf based on a SBuf& or char*
the parameter used may be pointing at the MemBlob memory buffer
indirectly without holding a separate ref-count lock to it.

If 'this' SBuf then requires reallocation for any reason the char* or
buffer pointer taken from the SBuf&, which is being manipulated may in
fact be left pointing at invalid memory.

Utilize a private Locker class to create relatively cheap ref-count locks
on the store_ MemBlob when this problem MAY occur. This Locker needs to
be used on all non-const SBuf methods accepting char* or SBuf& argument.
---

diff --git a/src/SBuf.cc b/src/SBuf.cc
index 79a04adba1..30ac2259a0 100644
--- a/src/SBuf.cc
+++ b/src/SBuf.cc
@@ -161,6 +161,7 @@ SBuf&
 SBuf::assign(const char *S, size_type n)
 {
     debugs(24, 6, id << " from c-string, n=" << n << ")");
+    Locker prevent_raw_memory_madness(this, S, n);
     clear();
     return append(S, n); //bounds checked in append()
 }
@@ -213,6 +214,7 @@ SBuf::clear()
 SBuf&
 SBuf::append(const SBuf &S)
 {
+    Locker prevent_raw_memory_madness(this, S.buf(), S.length());
     return lowAppend(S.buf(), S.length());
 }
 
@@ -224,6 +226,7 @@ SBuf::append(const char * S, size_type Ssize)
     if (Ssize == SBuf::npos)
         Ssize = strlen(S);
     debugs(24, 7, "from c-string to id " << id);
+    Locker prevent_raw_memory_madness(this, S, Ssize);
     // coverity[access_dbuff_in_call]
     return lowAppend(S, Ssize);
 }
@@ -237,6 +240,10 @@ SBuf::append(const char c)
 SBuf&
 SBuf::Printf(const char *fmt, ...)
 {
+    // with printf() an arg might be a dangerous char*
+    // NP: cant rely on vappendf() Locker because of clear()
+    Locker prevent_raw_memory_madness(this, buf(), length());
+
     va_list args;
     va_start(args, fmt);
     clear();
@@ -263,6 +270,9 @@ SBuf::vappendf(const char *fmt, va_list vargs)
     //reserve twice the format-string size, it's a likely heuristic
     size_type requiredSpaceEstimate = strlen(fmt)*2;
 
+    // with appendf() an arg might be a dangerous char*
+    Locker prevent_raw_memory_madness(this, buf(), length());
+
     char *space = rawSpace(requiredSpaceEstimate);
 #ifdef VA_COPY
     va_list ap;
diff --git a/src/SBuf.h b/src/SBuf.h
index dcbe587720..7258b828c8 100644
--- a/src/SBuf.h
+++ b/src/SBuf.h
@@ -667,6 +667,20 @@ public:
     // TODO: possibly implement a replace() call
 private:
 
+    class Locker
+    {
+    public:
+        Locker(SBuf *parent, const char *Q, size_t len) : locket(nullptr) {
+            // lock if Q intersects the parents buffer area
+            const MemBlob *P = parent->store_.getRaw();
+            if ( (Q+len) >= P->mem && Q <= (P->mem + P->capacity) )
+                locket = P;
+        }
+    private:
+        MemBlob::Pointer locket;
+    };
+    friend class Locker;
+
     MemBlob::Pointer store_; ///< memory block, possibly shared with other SBufs
     size_type off_; ///< our content start offset from the beginning of shared store_
     size_type len_; ///< number of our content bytes in shared store_