Parser: limit body part nesting depth and bound boundary searches

src/vmime/body.cpp

@@ -54,10 +54,23 @@ body::~body() {
 }
 
 
+// Returns the number of parts above the specified part
+static size_t getPartDepth(const bodyPart* part) {
+
+	size_t depth = 0;
+
+	for (const bodyPart* p = part->getParentPart() ; p ; p = p->getParentPart()) {
+		++depth;
+	}
+
+	return depth;
+}
+
+
 /*
  * boundaryStart: will become the index for "\r\n--marker"
  * boundaryEnd:   will become the index after "marker", i.e. index for potential trailing "--", "\r\n", etc.
- * return value:  index for "marker"
+ * return value:  index for "marker", or npos if not found before 'end'
  */
 // static
 size_t body::findNextBoundaryPosition(
@@ -69,56 +82,45 @@ size_t body::findNextBoundaryPosition(
 	size_t* boundaryEnd
 ) {
 
-	size_t pos = position;
+	// Boundary should be at the beginning of a line, and should start
+	// with "--". The search starts one byte before 'position', as the
+	// line break may be just before it (eg. at the end of the headers).
+	const string delimiter = "\n--" + boundary;
 
-	for (; pos != npos && pos < end; ++pos) {
+	for (size_t pos = (position == 0 ? 0 : position - 1) ; pos < end ; ++pos) {
 
-		pos = parser->findNext(boundary, pos);
+		pos = parser->findNext(delimiter, pos, end);
 
 		if (pos == npos) {
 			break;  // not found
 		}
 
-		if (pos == 0) {
-			// Boundary is a prefix of another, continue the search (same for the other "continue"s)
-			continue;
-		}
-
-		// Ensure the bytes before boundary are "[LF]--": boundary should be
-		// at the beginning of a line, and should start with "--"
-		if (pos < 3) {
-			continue;
-		}
-		parser->seek(pos - 3);
-		if (!parser->matchBytes("\n--", 3)) {
-			continue;
-		}
-
-		parser->seek(pos + boundary.length());
+		parser->seek(pos + delimiter.length());
 
 		const byte_t next = parser->peekByte();
 
-		// Boundary should be followed by a new line or a dash
+		// Boundary should be followed by a new line or a dash; if not, it is
+		// a prefix of another boundary: continue the search
 		if (!isspace(next) && next != '-') {
 			continue;
 		}
 
 		// Get rid of the "[CR]" just before "[LF]--", if any
 		size_t backwards = 0;
-		if (pos >= 4) {
-			parser->seek(pos - 4);
+		if (pos >= 1) {
+			parser->seek(pos - 1);
 			if (parser->peekByte() == '\r') {
 				++backwards;
 			}
 		}
 
-		*boundaryStart = pos - backwards - 3;
-		*boundaryEnd = pos + boundary.length();
+		*boundaryStart = pos - backwards;
+		*boundaryEnd = pos + delimiter.length();
 
-		return pos;
+		return pos + 3;
 	}
 
-	return pos;
+	return npos;
 }
 
 
@@ -158,7 +160,10 @@ void body::parseImpl(
 
 		const mediaType type = *ctf->getValue <mediaType>();
 
-		if (type.getType() == mediaTypes::MULTIPART) {
+		// Do not split parts nested too deeply, as each level adds to the stack
+		// and scans the contents of its sub-parts: keep their contents as is
+		if (type.getType() == mediaTypes::MULTIPART &&
+		    getPartDepth(m_part) < ctx.getMaxNestingDepth()) {
 
 			isMultipart = true;
 
@@ -180,7 +185,7 @@ void body::parseImpl(
 
 				} else {
 
-					pos = parser->findNext("\n--", position);
+					pos = parser->findNext("\n--", position, end);
 
 					if ((pos != npos) && (pos + 3 < end)) {
 						pos += 3;  // skip \n--
@@ -323,16 +328,19 @@ void body::parseImpl(
 
 			partStart = boundaryEnd;
 
-			// Find the next boundary
-			pos = findNextBoundaryPosition(
-				parser, boundary, boundaryEnd, end, &boundaryStart, &boundaryEnd
-			);
+			// Find the next boundary (none after the last one, which would scan the epilog)
+			if (!lastPart) {
+				pos = findNextBoundaryPosition(
+					parser, boundary, boundaryEnd, end, &boundaryStart, &boundaryEnd
+				);
+			}
 		}
 
 		m_contents = make_shared <emptyContentHandler>();
 
-		// Last part was not found: recover from missing boundary
-		if (!lastPart && pos == npos) {
+		// Last part was not found: recover from missing boundary (but do not
+		// create an empty part when the last boundary is at the very end)
+		if (!lastPart && pos == npos && partStart < end) {
 
 			shared_ptr <bodyPart> part = m_part->createChildPart();
 

src/vmime/component.cpp

@@ -90,8 +90,14 @@ void component::parse(
 
 	} else {
 
+		// Do not wrap the stream again if it already is a parser adapter (eg. when
+		// parsing sub-parts), as each level would add an indirection to all accesses
 		shared_ptr <utility::parserInputStreamAdapter> parser =
-			make_shared <utility::parserInputStreamAdapter>(seekableStream);
+			dynamicCast <utility::parserInputStreamAdapter>(seekableStream);
+
+		if (!parser) {
+			parser = make_shared <utility::parserInputStreamAdapter>(seekableStream);
+		}
 
 		parseImpl(ctx, parser, position, end, newPosition);
 	}

src/vmime/parsingContext.cpp

@@ -35,7 +35,8 @@ parsingContext::parsingContext()
 
 parsingContext::parsingContext(const parsingContext& ctx)
 	: context(ctx),
-	  m_headerParseErrorRecovery(vmime::headerParseRecoveryMethod::SKIP_LINE) {
+	  m_headerParseErrorRecovery(vmime::headerParseRecoveryMethod::SKIP_LINE),
+	  m_maxNestingDepth(ctx.m_maxNestingDepth) {
 
 }
 
@@ -85,4 +86,16 @@ void parsingContext::setUseMyHostname(bool useMyHostname) {
 }
 
 
+size_t parsingContext::getMaxNestingDepth() const {
+
+	return m_maxNestingDepth;
+}
+
+
+void parsingContext::setMaxNestingDepth(const size_t maxDepth) {
+
+	m_maxNestingDepth = maxDepth;
+}
+
+
 } // vmime

src/vmime/parsingContext.hpp

@@ -101,6 +101,23 @@ class VMIME_EXPORT parsingContext : public context {
 	  */
 	void setUseMyHostname(bool useMyHostname);
 
+	/** Returns the maximum nesting depth of body parts.
+	  *
+	  * @return maximum nesting depth of body parts
+	  */
+	size_t getMaxNestingDepth() const;
+
+	/** Sets the maximum nesting depth of body parts. The root part (ie. the
+	  * message) has a depth of 0, its sub-parts have a depth of 1, and so on.
+	  * A multipart body part which is at the maximum depth is not split into
+	  * sub-parts: its body contents are kept as is, as for a non-multipart
+	  * body part. This protects against messages crafted to exhaust the stack
+	  * or the CPU. The default is 100.
+	  *
+	  * @param maxDepth maximum nesting depth of body parts
+	  */
+	void setMaxNestingDepth(const size_t maxDepth);
+
 protected:
 
 	headerParseRecoveryMethod::headerLineError m_headerParseErrorRecovery;
@@ -120,6 +137,10 @@ class VMIME_EXPORT parsingContext : public context {
 	  *  for header fields when one is not present.
 	  */
 	bool m_useMyHostname{true};
+
+	/** Maximum nesting depth of body parts.
+	  */
+	size_t m_maxNestingDepth{100};
 };
 
 

src/vmime/utility/parserInputStreamAdapter.cpp

@@ -92,13 +92,14 @@ const string parserInputStreamAdapter::extract(const size_t begin, const size_t
 
 size_t parserInputStreamAdapter::findNext(
 	const string& token,
-	const size_t startPosition
+	const size_t startPosition,
+	const size_t endPosition
 ) {
 
-	static const unsigned int BUFFER_SIZE = 4096;
+	static const size_t BUFFER_SIZE = 4096;
 
 	// Token must not be longer than BUFFER_SIZE/2
-	if (token.empty() || token.length() > BUFFER_SIZE / 2) {
+	if (token.empty() || token.length() > BUFFER_SIZE / 2 || startPosition >= endPosition) {
 		return npos;
 	}
 
@@ -109,54 +110,66 @@ size_t parserInputStreamAdapter::findNext(
 	try {
 
 		byte_t findBuffer[BUFFER_SIZE];
-		byte_t* findBuffer1 = findBuffer;
-		byte_t* findBuffer2 = findBuffer + (BUFFER_SIZE / 2);
-
 		size_t findBufferLen = 0;
-		size_t findBufferOffset = 0;
 
-		bool isEOF = false;
+		// Position of the first byte of the buffer in the stream
+		size_t findBufferOffset = startPosition;
 
-		// Fill in initial buffer
-		findBufferLen = read(findBuffer, BUFFER_SIZE);
+		// Number of bytes which can still be read before reaching 'endPosition'
+		size_t remaining = endPosition - startPosition;
 
-		while (findBufferLen != 0) {
+		// Start with small reads, and double their size after each one: the
+		// cost of a search is then proportional to the distance to the token,
+		// which matters when the caller rejects a match and searches again
+		size_t readSize = 64;
 
-			// Find token
-			for (byte_t *begin = findBuffer, *end = findBuffer + findBufferLen - token.length() ;
-			     begin <= end ; ++begin) {
+		while (true) {
 
-				if (begin[0] == token[0] &&
-				    (token.length() == 1 ||
-				     memcmp(static_cast <const void *>(&begin[1]),
-				            static_cast <const void *>(token.data() + 1),
-				            token.length() - 1) == 0)) {
+			const size_t bytesRead = read(
+				findBuffer + findBufferLen,
+				std::min(std::min(readSize, BUFFER_SIZE - findBufferLen), remaining)
+			);
 
-					seek(initialPos);
+			findBufferLen += bytesRead;
+			remaining -= bytesRead;
 
-					return startPosition + findBufferOffset + (begin - findBuffer);
-				}
-			}
+			// Find token
+			if (findBufferLen >= token.length()) {
 
-			// Rotate buffer
-			memcpy(findBuffer1, findBuffer2, (BUFFER_SIZE / 2));
+				const byte_t* const last = findBuffer + findBufferLen - token.length();
 
-			// Read more bytes
-			if (findBufferLen < BUFFER_SIZE && (eof() || isEOF)) {
+				for (const byte_t* begin = findBuffer ; begin <= last ; ++begin) {
 
-				break;
+					begin = static_cast <const byte_t*>(memchr(begin, token[0], last - begin + 1));
+
+					if (!begin) {
+						break;
+					}
 
-			} else {
+					if (memcmp(begin + 1, token.data() + 1, token.length() - 1) == 0) {
 
-				const size_t bytesRead = read(findBuffer2, BUFFER_SIZE / 2);
+						seek(initialPos);
 
-				if (bytesRead == 0) {
-					isEOF = true;
+						return findBufferOffset + (begin - findBuffer);
+					}
 				}
+			}
 
-				findBufferLen = (BUFFER_SIZE / 2) + bytesRead;
-				findBufferOffset += (BUFFER_SIZE / 2);
+			// End of stream, or 'endPosition' reached
+			if (bytesRead == 0 || remaining == 0) {
+				break;
 			}
+
+			// Keep the bytes which have not been tested yet as the start of
+			// the token: they may be the beginning of an occurrence
+			const size_t keep = std::min(findBufferLen, token.length() - 1);
+
+			memmove(findBuffer, findBuffer + findBufferLen - keep, keep);
+
+			findBufferOffset += findBufferLen - keep;
+			findBufferLen = keep;
+
+			readSize = std::min(readSize * 2, BUFFER_SIZE);
 		}
 
 		seek(initialPos);

src/vmime/utility/parserInputStreamAdapter.hpp

@@ -157,7 +157,19 @@ class VMIME_EXPORT parserInputStreamAdapter : public seekableInputStream {
 		return pos - initialPos;
 	}
 
-	size_t findNext(const string& token, const size_t startPosition = 0);
+	/** Finds the next occurrence of a token. The current position is not updated.
+	  *
+	  * @param token token to search for
+	  * @param startPosition position at which to start the search
+	  * @param endPosition the token must end at or before this position (or
+	  * at end of the stream, whichever comes first)
+	  * @return position of the first byte of the token, or npos if not found
+	  */
+	size_t findNext(
+		const string& token,
+		const size_t startPosition = 0,
+		const size_t endPosition = npos
+	);
 
 private: