diff --git a/src/vmime/body.cpp b/src/vmime/body.cpp index 1ba7d420..5e0edf05 100644 --- a/src/vmime/body.cpp +++ b/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 (); - 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 (); - // 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 part = m_part->createChildPart(); diff --git a/src/vmime/component.cpp b/src/vmime/component.cpp index e5e0370e..618264f5 100644 --- a/src/vmime/component.cpp +++ b/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 parser = - make_shared (seekableStream); + dynamicCast (seekableStream); + + if (!parser) { + parser = make_shared (seekableStream); + } parseImpl(ctx, parser, position, end, newPosition); } diff --git a/src/vmime/parsingContext.cpp b/src/vmime/parsingContext.cpp index e80eab78..753076a4 100644 --- a/src/vmime/parsingContext.cpp +++ b/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 diff --git a/src/vmime/parsingContext.hpp b/src/vmime/parsingContext.hpp index 95c51959..dbcf8962 100644 --- a/src/vmime/parsingContext.hpp +++ b/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}; }; diff --git a/src/vmime/utility/parserInputStreamAdapter.cpp b/src/vmime/utility/parserInputStreamAdapter.cpp index 98bbe607..3635c059 100644 --- a/src/vmime/utility/parserInputStreamAdapter.cpp +++ b/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 (&begin[1]), - static_cast (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 (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); diff --git a/src/vmime/utility/parserInputStreamAdapter.hpp b/src/vmime/utility/parserInputStreamAdapter.hpp index e448d29c..22dee79e 100644 --- a/src/vmime/utility/parserInputStreamAdapter.hpp +++ b/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: diff --git a/tests/parser/bodyPartTest.cpp b/tests/parser/bodyPartTest.cpp index 3aaadd03..b82f27b4 100644 --- a/tests/parser/bodyPartTest.cpp +++ b/tests/parser/bodyPartTest.cpp @@ -40,6 +40,10 @@ VMIME_TEST_SUITE_BEGIN(bodyPartTest) VMIME_TEST(testTextUsageForQPEncoding) VMIME_TEST(testParseVeryBigMessage) VMIME_TEST(testParseBoundaryPrefix) + VMIME_TEST(testParseMissingLastBoundaryAtEnd) + VMIME_TEST(testParseMissingLastBoundaryInSubPart) + VMIME_TEST(testParseMaxNestingDepth) + VMIME_TEST(testParseDeeplyNestedParts) VMIME_TEST_LIST_END @@ -66,6 +70,32 @@ VMIME_TEST_SUITE_BEGIN(bodyPartTest) return oss.str(); } + // Returns a message whose innermost part, which contains "LEAF", + // is at the specified depth + static const vmime::string buildNestedMessage(const size_t depth) { + + std::ostringstream oss; + + for (size_t i = 0 ; i < depth ; ++i) { + + oss << "Content-Type: multipart/mixed; boundary=\"b" << i << "\"\r\n" + << "\r\n" + << "--b" << i << "\r\n"; + } + + oss << "Content-Type: text/plain\r\n" + << "\r\n" + << "LEAF"; + + for (size_t i = depth ; i > 0 ; --i) { + oss << "\r\n--b" << (i - 1) << "--"; + } + + oss << "\r\n"; + + return oss.str(); + } + void testParse() { @@ -411,4 +441,105 @@ VMIME_TEST_SUITE_BEGIN(bodyPartTest) VASSERT_EQ("part2-body", "P2", extractContents(relbd->getPartAt(1)->getBody()->getContents())); } + void testParseMissingLastBoundaryAtEnd() { + + // No empty part should be created after the last boundary + vmime::string str = + "Content-Type: multipart/mixed; boundary=\"MY-BOUNDARY\"" + "\r\n\r\n" + "--MY-BOUNDARY\r\nHEADER1\r\n\r\nBODY1\r\n" + "--MY-BOUNDARY\r\n"; + + vmime::bodyPart p; + p.parse(str); + + VASSERT_EQ("count", 1, p.getBody()->getPartCount()); + + VASSERT_EQ("part1-body", "BODY1", extractContents(p.getBody()->getPartAt(0)->getBody()->getContents())); + } + + void testParseMissingLastBoundaryInSubPart() { + + // The boundary of the sub-part found after the end of the + // enclosing part must not be taken into account + vmime::string str = + "Content-Type: multipart/mixed; boundary=\"OUTER\"\r\n" + "\r\n" + "--OUTER\r\n" + "Content-Type: multipart/mixed; boundary=\"INNER\"\r\n" + "\r\n" + "--INNER\r\n" + "\r\n" + "BODY1\r\n" + "--OUTER--\r\n" + "--INNER\r\n" + "\r\n" + "EPILOG\r\n"; + + vmime::bodyPart p; + p.parse(str); + + VASSERT_EQ("count", 1, p.getBody()->getPartCount()); + + vmime::shared_ptr inner = p.getBody()->getPartAt(0)->getBody(); + + VASSERT_EQ("inner-count", 1, inner->getPartCount()); + VASSERT_EQ("inner-part-body", "BODY1", extractContents(inner->getPartAt(0)->getBody()->getContents())); + } + + void testParseMaxNestingDepth() { + + vmime::parsingContext ctx; + ctx.setMaxNestingDepth(3); + + vmime::bodyPart p; + p.parse(ctx, buildNestedMessage(4)); + + vmime::shared_ptr body = p.getBody(); + + for (size_t depth = 1 ; depth <= 3 ; ++depth) { + + VASSERT_EQ("count", 1, body->getPartCount()); + + body = body->getPartAt(0)->getBody(); + } + + // Part at maximum depth is not split, contents are kept as is + VASSERT_EQ("max-depth-count", 0, body->getPartCount()); + VASSERT_EQ( + "max-depth-contents", + "--b3\r\nContent-Type: text/plain\r\n\r\nLEAF\r\n--b3--", + extractContents(body->getContents()) + ); + + VASSERT("generate", p.generate().find("\r\nLEAF\r\n--b3--\r\n") != vmime::string::npos); + } + + void testParseDeeplyNestedParts() { + + // Would exhaust the stack if depth was not limited + vmime::parsingContext ctx; + + VASSERT_EQ("default-max-depth", 100, ctx.getMaxNestingDepth()); + + vmime::bodyPart p; + p.parse(ctx, buildNestedMessage(10000)); + + vmime::shared_ptr body = p.getBody(); + + for (size_t depth = 1 ; depth <= 100 ; ++depth) { + + VASSERT_EQ("count", 1, body->getPartCount()); + + body = body->getPartAt(0)->getBody(); + } + + VASSERT_EQ("max-depth-count", 0, body->getPartCount()); + + const vmime::string contents = extractContents(body->getContents()); + + VASSERT_EQ("max-depth-contents-begin", "--b100\r\n", contents.substr(0, 8)); + VASSERT_EQ("max-depth-contents-end", "\r\n--b100--", contents.substr(contents.length() - 10)); + } + VMIME_TEST_SUITE_END diff --git a/tests/utility/parserInputStreamAdapterTest.cpp b/tests/utility/parserInputStreamAdapterTest.cpp index 4bc310c9..71ed533c 100644 --- a/tests/utility/parserInputStreamAdapterTest.cpp +++ b/tests/utility/parserInputStreamAdapterTest.cpp @@ -26,13 +26,145 @@ #include "vmime/utility/parserInputStreamAdapter.hpp" +// Seekable stream which returns at most a few bytes on each read +class shortReadInputStream : public vmime::utility::seekableInputStream { + +public: + + shortReadInputStream(const vmime::string& str, const size_t maxRead) + : m_stream(str), m_maxRead(maxRead) { + + } + + bool eof() const { return m_stream.eof(); } + void reset() { m_stream.reset(); } + size_t skip(const size_t count) { return m_stream.skip(count); } + size_t getPosition() const { return m_stream.getPosition(); } + void seek(const size_t pos) { m_stream.seek(pos); } + + size_t read(vmime::byte_t* const data, const size_t count) { + + return m_stream.read(data, std::min(count, m_maxRead)); + } + +private: + + vmime::utility::inputStreamStringAdapter m_stream; + const size_t m_maxRead; +}; + + VMIME_TEST_SUITE_BEGIN(parserInputStreamAdapterTest) VMIME_TEST_LIST_BEGIN VMIME_TEST(testEndlessLoopBufferSize) + VMIME_TEST(testFindNext) + VMIME_TEST(testFindNextEndPosition) + VMIME_TEST(testFindNextAllPositions) + VMIME_TEST(testFindNextLongToken) + VMIME_TEST(testFindNextShortReads) VMIME_TEST_LIST_END + static vmime::shared_ptr createParser( + const vmime::string& str + ) { + + return vmime::make_shared ( + vmime::make_shared (str) + ); + } + + void testFindNext() { + + vmime::shared_ptr parser = + createParser("token-before-token-after-token"); + + parser->seek(7); + + VASSERT_EQ("1", 0, parser->findNext("token")); + VASSERT_EQ("2", 13, parser->findNext("token", 1)); + VASSERT_EQ("3", 13, parser->findNext("token", 13)); + VASSERT_EQ("4", 25, parser->findNext("token", 14)); + VASSERT_EQ("5", vmime::string::npos, parser->findNext("token", 26)); + VASSERT_EQ("6", vmime::string::npos, parser->findNext("not-found")); + VASSERT_EQ("7", vmime::string::npos, parser->findNext("")); + + // Current position is not updated + VASSERT_EQ("8", 7, parser->getPosition()); + } + + void testFindNextEndPosition() { + + vmime::shared_ptr parser = + createParser("0123token9token"); + + // Token must end at or before end position + VASSERT_EQ("1", 4, parser->findNext("token", 0, 9)); + VASSERT_EQ("2", vmime::string::npos, parser->findNext("token", 0, 8)); + VASSERT_EQ("3", vmime::string::npos, parser->findNext("token", 5, 14)); + VASSERT_EQ("4", 10, parser->findNext("token", 5, 15)); + VASSERT_EQ("5", 10, parser->findNext("token", 5, 1000)); + + VASSERT_EQ("6", vmime::string::npos, parser->findNext("token", 4, 4)); + VASSERT_EQ("7", vmime::string::npos, parser->findNext("token", 5, 4)); + } + + void testFindNextAllPositions() { + + // Larger than the sum of the first internal reads + static const size_t LENGTH = 4300; + + // Token at each position, including across internal reads, + // and with partial matches everywhere before it + vmime::string str(LENGTH, 'a'); + + for (size_t pos = 0 ; pos + 3 <= LENGTH ; ++pos) { + + str.replace(pos, 3, "aab"); + + vmime::shared_ptr parser = createParser(str); + + VASSERT_EQ("found", pos, parser->findNext("aab")); + VASSERT_EQ("end", pos, parser->findNext("aab", 0, pos + 3)); + VASSERT_EQ("before-end", vmime::string::npos, parser->findNext("aab", 0, pos + 2)); + + str.replace(pos, 3, "aaa"); + } + } + + void testFindNextLongToken() { + + static const size_t BUFFER_SIZE = 4096; // same as in parserInputStreamAdapter::findNext() + + const vmime::string token = vmime::string(BUFFER_SIZE / 2 - 1, 'a') + "b"; + + for (size_t pos = 0 ; pos < 3 * BUFFER_SIZE ; pos += 997) { + + vmime::shared_ptr parser = + createParser(vmime::string(pos, 'a') + token + vmime::string(100, 'a')); + + VASSERT_EQ("found", pos, parser->findNext(token)); + } + + // Token longer than BUFFER_SIZE/2 is not supported + VASSERT_EQ("too-long", vmime::string::npos, + createParser(vmime::string(BUFFER_SIZE, 'a'))->findNext(vmime::string(BUFFER_SIZE / 2 + 1, 'a'))); + } + + void testFindNextShortReads() { + + const vmime::string str = vmime::string(10000, 'X') + "token" + vmime::string(100, 'X'); + + vmime::shared_ptr parser = + vmime::make_shared ( + vmime::make_shared (str, 7) + ); + + VASSERT_EQ("found", 10000, parser->findNext("token")); + VASSERT_EQ("not-found", vmime::string::npos, parser->findNext("token", 10001)); + } + void testEndlessLoopBufferSize() { static const unsigned int BUFFER_SIZE = 4096; // same as in parserInputStreamAdapter::findNext()