From: Amos Jeffries <squid3@treenet.co.nz>
Date: Fri, 30 Oct 2015 18:26:41 +0000 (-0700)
Subject: Add Locker friend class to SBuf for protection against memory issues
X-Git-Tag: SQUID_3_5_11~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5773237ee528d5898bb04b017b6ac5e9095079c0;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 e87df17fda..afbd709dbb 100644
--- a/src/SBuf.cc
+++ b/src/SBuf.cc
@@ -149,6 +149,7 @@ SBuf::assign(const SBuf &S)
 SBuf&
 SBuf::assign(const char *S, size_type n)
 {
+    const Locker blobKeeper(this, S);
     debugs(24, 6, id << " from c-string, n=" << n << ")");
     clear();
     return append(S, n); //bounds checked in append()
@@ -202,12 +203,14 @@ SBuf::clear()
 SBuf&
 SBuf::append(const SBuf &S)
 {
+    const Locker blobKeeper(this, S.buf());
     return lowAppend(S.buf(), S.length());
 }
 
 SBuf &
 SBuf::append(const char * S, size_type Ssize)
 {
+    const Locker blobKeeper(this, S);
     if (S == NULL)
         return *this;
     if (Ssize == SBuf::npos)
@@ -226,6 +229,10 @@ SBuf::append(const char c)
 SBuf&
 SBuf::Printf(const char *fmt, ...)
 {
+    // with printf() the fmt or an arg might be a dangerous char*
+    // NP: cant rely on vappendf() Locker because of clear()
+    const Locker blobKeeper(this, buf());
+
     va_list args;
     va_start(args, fmt);
     clear();
@@ -247,6 +254,9 @@ SBuf::appendf(const char *fmt, ...)
 SBuf&
 SBuf::vappendf(const char *fmt, va_list vargs)
 {
+    // with (v)appendf() the fmt or an arg might be a dangerous char*
+    const Locker blobKeeper(this, buf());
+
     Must(fmt != NULL);
     int sz = 0;
     //reserve twice the format-string size, it's a likely heuristic
@@ -785,6 +795,10 @@ SBuf::findFirstNotOf(const CharacterSet &set, size_type startPos) const
 int
 SBuf::scanf(const char *format, ...)
 {
+    // with the format or an arg might be a dangerous char*
+    // that gets invalidated by c_str()
+    const Locker blobKeeper(this, buf());
+
     va_list arg;
     int rv;
     ++stats.scanf;
diff --git a/src/SBuf.h b/src/SBuf.h
index ef77733dc1..e9be770c99 100644
--- a/src/SBuf.h
+++ b/src/SBuf.h
@@ -545,6 +545,27 @@ public:
     // TODO: possibly implement a replace() call
 private:
 
+    /**
+     * Keeps SBuf's MemBlob alive in a blob-destroying context where
+     * a seemingly unrelated memory pointer may belong to the same blob.
+     * For [an extreme] example, consider: a.append(a).
+     * Compared to an SBuf temporary, this class is optimized to
+     * preserve blobs only if needed and to reduce debugging noise.
+     */
+    class Locker
+    {
+    public:
+        Locker(SBuf *parent, const char *otherBuffer) {
+            // lock if otherBuffer intersects the parents buffer area
+            const MemBlob *blob = parent->store_.getRaw();
+            if (blob->mem <= otherBuffer && otherBuffer < (blob->mem + blob->capacity))
+                locket = blob;
+        }
+    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_