Bugfix: Possible Out of Bounds Write in StreamNameIO and BlockNameIO #15

Issue #15, the encodeName functions fail to verify buffer length can
  store encoded filenames
For good measure and interface consistency, also check decodeName fnc

encfs/BlockNameIO.cpp

@@ -135,14 +135,17 @@ int BlockNameIO::maxDecodedNameLen(int encodedNameLen) const {
 }
 
 int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv,
-                            char *encodedName) const {
+                            char *encodedName, int bufferLength) const {
+
   // copy the data into the encoding buffer..
+  rAssert(length <= (bufferLength - 2));
   memcpy(encodedName + 2, plaintextName, length);
 
   // Pad encryption buffer to block boundary..
   int padding = _bs - length % _bs;
   if (padding == 0) padding = _bs;  // padding a full extra block!
 
+  rAssert(padding <= (bufferLength - length - 2));
   memset(encodedName + length + 2, (unsigned char)padding, padding);
 
   // store the IV before it is modified by the MAC call.
@@ -182,7 +185,7 @@ int BlockNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv,
 }
 
 int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv,
-                            char *plaintextName) const {
+                            char *plaintextName, int bufferLength) const {
   int decLen256 =
       _caseSensitive ? B32ToB256Bytes(length) : B64ToB256Bytes(length);
   int decodedStreamLen = decLen256 - 2;
@@ -225,6 +228,7 @@ int BlockNameIO::decodeName(const char *encodedName, int length, uint64_t *iv,
   }
 
   // copy out the result..
+  rAssert(finalSize < bufferLength);
   memcpy(plaintextName, tmpBuf + 2, finalSize);
   plaintextName[finalSize] = '\0';
 

encfs/BlockNameIO.h

@@ -55,9 +55,9 @@ class BlockNameIO : public NameIO {
 
  protected:
   virtual int encodeName(const char *plaintextName, int length, uint64_t *iv,
-                         char *encodedName) const;
+                         char *encodedName, int bufferLength) const;
   virtual int decodeName(const char *encodedName, int length, uint64_t *iv,
-                         char *plaintextName) const;
+                         char *plaintextName, int bufferLength) const;
 
  private:
   int _interface;

encfs/NameIO.cpp

@@ -142,7 +142,7 @@ bool NameIO::getReverseEncryption() const { return reverseEncryption; }
 std::string NameIO::recodePath(const char *path,
                                int (NameIO::*_length)(int) const,
                                int (NameIO::*_code)(const char *, int,
-                                                    uint64_t *, char *) const,
+                                                    uint64_t *, char *, int) const,
                                uint64_t *iv) const {
   string output;
 
@@ -166,11 +166,12 @@ std::string NameIO::recodePath(const char *path,
       // figure out buffer sizes
       int approxLen = (this->*_length)(len);
       if (approxLen <= 0) throw ERROR("Filename too small to decode");
+	  int bufSize = 0;
 
-      BUFFER_INIT(codeBuf, 32, (unsigned int)approxLen + 1)
+      BUFFER_INIT_S(codeBuf, 32, (unsigned int)approxLen + 1, bufSize)
-
+	  
       // code the name
-      int codedLen = (this->*_code)(path, len, iv, codeBuf);
+      int codedLen = (this->*_code)(path, len, iv, codeBuf, bufSize);
       rAssert(codedLen <= approxLen);
       rAssert(codeBuf[codedLen] == '\0');
       path += len;
@@ -217,21 +218,22 @@ std::string NameIO::decodePath(const char *path, uint64_t *iv) const {
   return getReverseEncryption() ? _encodePath(path, iv) : _decodePath(path, iv);
 }
 
-int NameIO::encodeName(const char *input, int length, char *output) const {
+int NameIO::encodeName(const char *input, int length, char *output, int bufferLength) const {
-  return encodeName(input, length, (uint64_t *)0, output);
+  return encodeName(input, length, (uint64_t *)0, output, bufferLength);
 }
 
-int NameIO::decodeName(const char *input, int length, char *output) const {
+int NameIO::decodeName(const char *input, int length, char *output, int bufferLength) const {
-  return decodeName(input, length, (uint64_t *)0, output);
+  return decodeName(input, length, (uint64_t *)0, output, bufferLength);
 }
 
 std::string NameIO::_encodeName(const char *plaintextName, int length) const {
   int approxLen = maxEncodedNameLen(length);
+  int bufSize = 0;
 
-  BUFFER_INIT(codeBuf, 32, (unsigned int)approxLen + 1)
+  BUFFER_INIT_S(codeBuf, 32, (unsigned int)approxLen + 1, bufSize)
-
+  
   // code the name
-  int codedLen = encodeName(plaintextName, length, 0, codeBuf);
+  int codedLen = encodeName(plaintextName, length, 0, codeBuf, bufSize);
   rAssert(codedLen <= approxLen);
   rAssert(codeBuf[codedLen] == '\0');
 
@@ -245,11 +247,12 @@ std::string NameIO::_encodeName(const char *plaintextName, int length) const {
 
 std::string NameIO::_decodeName(const char *encodedName, int length) const {
   int approxLen = maxDecodedNameLen(length);
+  int bufSize = 0;
 
-  BUFFER_INIT(codeBuf, 32, (unsigned int)approxLen + 1)
+  BUFFER_INIT_S(codeBuf, 32, (unsigned int)approxLen + 1, bufSize)
-
+  
   // code the name
-  int codedLen = decodeName(encodedName, length, 0, codeBuf);
+  int codedLen = decodeName(encodedName, length, 0, codeBuf, bufSize);
   rAssert(codedLen <= approxLen);
   rAssert(codeBuf[codedLen] == '\0');
 

encfs/NameIO.h

@@ -83,19 +83,19 @@ class NameIO {
 
  protected:
   virtual int encodeName(const char *plaintextName, int length,
-                         char *encodedName) const;
+                         char *encodedName, int bufferLength) const;
   virtual int decodeName(const char *encodedName, int length,
-                         char *plaintextName) const;
+                         char *plaintextName, int bufferLength) const;
 
   virtual int encodeName(const char *plaintextName, int length, uint64_t *iv,
-                         char *encodedName) const = 0;
+                         char *encodedName, int bufferLength) const = 0;
   virtual int decodeName(const char *encodedName, int length, uint64_t *iv,
-                         char *plaintextName) const = 0;
+                         char *plaintextName, int bufferLength) const = 0;
 
  private:
   std::string recodePath(const char *path, int (NameIO::*codingLen)(int) const,
                          int (NameIO::*codingFunc)(const char *, int,
-                                                   uint64_t *, char *) const,
+                                                   uint64_t *, char *, int) const,
                          uint64_t *iv) const;
 
   std::string _encodePath(const char *plaintextPath, uint64_t *iv) const;
@@ -108,11 +108,11 @@ class NameIO {
 };
 
 /*
-    Helper macros for creating temporary buffers with an optimization that
+	Helper macros for creating temporary buffers with an optimization that
-    below a given size (OptimizedSize) is allocated on the stack, and when a
+	below a given size (OptimizedSize) is allocated on the stack, and when a
-    larger size is requested it is allocated on the heap.
+	larger size is requested it is allocated on the heap.
 
-    BUFFER_RESET should be called for the same name as BUFFER_INIT
+	BUFFER_RESET should be called for the same name as BUFFER_INIT
 */
 #define BUFFER_INIT(Name, OptimizedSize, Size)          \
   char Name##_Raw[OptimizedSize];                       \
@@ -120,6 +120,16 @@ class NameIO {
   if (sizeof(Name##_Raw) < Size) Name = new char[Size]; \
   memset(Name, 0, Size);
 
+#define BUFFER_INIT_S(Name, OptimizedSize, Size, BufSize) \
+  char Name##_Raw[OptimizedSize];                         \
+  BufSize = sizeof(Name##_Raw);                           \
+  char *Name = Name##_Raw;                                \
+  if (sizeof(Name##_Raw) < Size) {                        \
+    Name = new char[Size];                                \
+    BufSize = Size;                                       \
+  }                                                       \
+  memset(Name, 0, Size);
+
 #define BUFFER_RESET(Name)    \
   do {                        \
     if (Name != Name##_Raw) { \

encfs/NullNameIO.cpp

@@ -56,16 +56,20 @@ int NullNameIO::maxDecodedNameLen(int encodedNameLen) const {
 }
 
 int NullNameIO::encodeName(const char *plaintextName, int length, uint64_t *iv,
-                           char *encodedName) const {
+                           char *encodedName, int bufferLength) const {
   (void)iv;
+  
+  rAssert(length <= bufferLength);
   memcpy(encodedName, plaintextName, length);
 
   return length;
 }
 
 int NullNameIO::decodeName(const char *encodedName, int length, uint64_t *iv,
-                           char *plaintextName) const {
+                           char *plaintextName, int bufferLength) const {
   (void)iv;
+
+  rAssert(length <= bufferLength);
   memcpy(plaintextName, encodedName, length);
 
   return length;

encfs/NullNameIO.h

@@ -23,6 +23,8 @@
 
 #include <stdint.h>
 
+#include "rlog/Error.h"
+#include "rlog/rlog.h"
 #include "Interface.h"
 #include "NameIO.h"
 
@@ -44,9 +46,9 @@ class NullNameIO : public NameIO {
 
  protected:
   virtual int encodeName(const char *plaintextName, int length, uint64_t *iv,
-                         char *encodedName) const;
+                         char *encodedName, int bufferLength) const;
   virtual int decodeName(const char *encodedName, int length, uint64_t *iv,
-                         char *plaintextName) const;
+                         char *plaintextName, int bufferLength) const;
 
  private:
 };

encfs/StreamNameIO.cpp

@@ -90,7 +90,7 @@ int StreamNameIO::maxDecodedNameLen(int encodedStreamLen) const {
 }
 
 int StreamNameIO::encodeName(const char *plaintextName, int length,
-                             uint64_t *iv, char *encodedName) const {
+                             uint64_t *iv, char *encodedName, int bufferLength) const {
   uint64_t tmpIV = 0;
   if (iv && _interface >= 2) tmpIV = *iv;
 
@@ -104,11 +104,13 @@ int StreamNameIO::encodeName(const char *plaintextName, int length,
     encodedName[0] = (mac >> 8) & 0xff;
     encodedName[1] = (mac)&0xff;
     encodeBegin = (unsigned char *)encodedName + 2;
+	rAssert(length <= (bufferLength - 2));
   } else {
     // encfs 0.x stored checksums at the end.
     encodedName[length] = (mac >> 8) & 0xff;
     encodedName[length + 1] = (mac)&0xff;
     encodeBegin = (unsigned char *)encodedName;
+	rAssert(length <= bufferLength);
   }
 
   // stream encode the plaintext bytes
@@ -126,10 +128,11 @@ int StreamNameIO::encodeName(const char *plaintextName, int length,
 }
 
 int StreamNameIO::decodeName(const char *encodedName, int length, uint64_t *iv,
-                             char *plaintextName) const {
+                             char *plaintextName, int bufferLength) const {
   rAssert(length > 2);
   int decLen256 = B64ToB256Bytes(length);
   int decodedStreamLen = decLen256 - 2;
+  rAssert(decodedStreamLen <= bufferLength);
 
   if (decodedStreamLen <= 0) throw ERROR("Filename too small to decode");
 

encfs/StreamNameIO.h

@@ -48,9 +48,9 @@ class StreamNameIO : public NameIO {
 
  protected:
   virtual int encodeName(const char *plaintextName, int length, uint64_t *iv,
-                         char *encodedName) const;
+                         char *encodedName, int bufferLength) const;
   virtual int decodeName(const char *encodedName, int length, uint64_t *iv,
-                         char *plaintextName) const;
+                         char *plaintextName, int bufferLength) const;
 
  private:
   int _interface;