Private/caolan/improvements

qa/libcmis/test-commons.cxx

@@ -64,11 +64,14 @@ class CommonsTest : public CppUnit::TestFixture
         // Methods that should never be called
         void objectTypeNocallTest();
 
+        void httpSessionCRLFInjectionTest();
+
         CPPUNIT_TEST_SUITE( CommonsTest );
         CPPUNIT_TEST( oauth2DataCopyTest );
         CPPUNIT_TEST( oauth2HandlerCopyTest );
         CPPUNIT_TEST( objectTypeCopyTest );
         CPPUNIT_TEST( objectTypeNocallTest );
+        CPPUNIT_TEST( httpSessionCRLFInjectionTest );
         CPPUNIT_TEST_SUITE_END( );
 };
 
@@ -107,32 +110,22 @@ void CommonsTest::oauth2HandlerCopyTest( )
 {
     OAuth2DataPtr data( new OAuth2Data ( "url", "token", "scope", "redirect",
                                          "clientid", "clientsecret" ) );
-    HttpSession session( "user", "pass" );
-    OAuth2Handler handler( &session, data );
+    HttpSession source( "user", "pass" );
+    HttpSession owner( "user", "pass" );
+    OAuth2Handler handler( &source, data );
     handler.m_access = "access";
     handler.m_refresh = "refresh";
     handler.m_oauth2Parser = &DummyOAuth2Parser;
 
-    {
-        OAuth2Handler copy;
-        copy = handler;
-
-        CPPUNIT_ASSERT_EQUAL( &session, copy.m_session );
-        CPPUNIT_ASSERT_EQUAL( data, copy.m_data );
-        CPPUNIT_ASSERT_EQUAL( handler.m_access, copy.m_access );
-        CPPUNIT_ASSERT_EQUAL( handler.m_refresh, copy.m_refresh );
-        CPPUNIT_ASSERT_EQUAL( &DummyOAuth2Parser, copy.m_oauth2Parser );
-    }
-
-    {
-        OAuth2Handler copy( handler );
+    OAuth2Handler copy( &owner, handler );
 
-        CPPUNIT_ASSERT_EQUAL( &session, copy.m_session );
-        CPPUNIT_ASSERT_EQUAL( data, copy.m_data );
-        CPPUNIT_ASSERT_EQUAL( handler.m_access, copy.m_access );
-        CPPUNIT_ASSERT_EQUAL( handler.m_refresh, copy.m_refresh );
-        CPPUNIT_ASSERT_EQUAL( &DummyOAuth2Parser, copy.m_oauth2Parser );
-    }
+    // m_session must follow the new owner, not the source, so that the
+    // copy keeps working after `source` is destroyed.
+    CPPUNIT_ASSERT_EQUAL( &owner, copy.m_session );
+    CPPUNIT_ASSERT_EQUAL( data, copy.m_data );
+    CPPUNIT_ASSERT_EQUAL( handler.m_access, copy.m_access );
+    CPPUNIT_ASSERT_EQUAL( handler.m_refresh, copy.m_refresh );
+    CPPUNIT_ASSERT_EQUAL( &DummyOAuth2Parser, copy.m_oauth2Parser );
 }
 
 static void assertObjectTypeEquals( const ObjectType& expected, const ObjectType& actual )
@@ -220,4 +213,21 @@ void CommonsTest::objectTypeNocallTest( )
     }
 }
 
+void CommonsTest::httpSessionCRLFInjectionTest( )
+{
+    HttpSession session( "user", "pass" );
+    std::string body( "hi" );
+    std::istringstream is( body );
+    try
+    {
+        session.httpPostRequest( "http://example.test/",
+                                 is,
+                                 "text/plain\r\nX-Injected: yes" );
+        CPPUNIT_FAIL( "httpPostRequest with CRLF in contentType should throw" );
+    }
+    catch ( const Exception& )
+    {
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION( CommonsTest );

qa/libcmis/test-decoder.cxx

@@ -61,6 +61,7 @@ class DecoderTest : public CppUnit::TestFixture
         void base64DecodePaddedBlockTest( );
         void base64DecodeNoEqualsPaddedBlockTest( );
         void base64DecodeSplitRunsTest( );
+        void base64DecodeExcessPaddingTest( );
 
         void base64EncodeSimpleBlockTest( );
         void base64EncodePaddedBlockTest( );
@@ -74,6 +75,7 @@ class DecoderTest : public CppUnit::TestFixture
         CPPUNIT_TEST( base64DecodePaddedBlockTest );
         CPPUNIT_TEST( base64DecodeNoEqualsPaddedBlockTest );
         CPPUNIT_TEST( base64DecodeSplitRunsTest );
+        CPPUNIT_TEST( base64DecodeExcessPaddingTest );
         CPPUNIT_TEST( base64EncodeSimpleBlockTest );
         CPPUNIT_TEST( base64EncodePaddedBlockTest );
         CPPUNIT_TEST( base64EncodeSplitRunsTest );
@@ -183,6 +185,18 @@ void DecoderTest::base64DecodeSplitRunsTest( )
     CPPUNIT_ASSERT_EQUAL( string( "pleasure." ), getActual( ) );
 }
 
+void DecoderTest::base64DecodeExcessPaddingTest( )
+{
+    // A corrupt stream with more '=' than a base64 block can have.  We must
+    // stay within the decodeBase64 buffer bounds.  Ignore the block and the
+    // previous valid run of "pleasure." must still get output.
+    data->setEncoding( BASE64_ENCODING );
+    string input( "cGxlYXN1cmUu========" );
+    data->decode( ( void* )input.c_str( ), 1, input.size( ) );
+    data->finish( );
+    CPPUNIT_ASSERT_EQUAL( string( "pleasure." ), getActual( ) );
+}
+
 void DecoderTest::base64EncodeSimpleBlockTest( )
 {
     data->setEncoding( BASE64_ENCODING );

qa/libcmis/test-jsonutils.cxx

@@ -56,6 +56,7 @@ class JsonTest : public CppUnit::TestFixture
 {
     public:
         void parseTest( );
+        void parseDeepNestingTest( );
         void parseTypeTest( );
         void createFromPropertyTest( );
         void createFromPropertiesTest( );
@@ -64,6 +65,7 @@ class JsonTest : public CppUnit::TestFixture
 
         CPPUNIT_TEST_SUITE( JsonTest );
         CPPUNIT_TEST( parseTest );
+        CPPUNIT_TEST( parseDeepNestingTest );
         CPPUNIT_TEST( parseTypeTest );
         CPPUNIT_TEST( createFromPropertyTest );
         CPPUNIT_TEST( createFromPropertiesTest );  
@@ -114,6 +116,18 @@ void JsonTest::parseTest( )
     CPPUNIT_ASSERT_EQUAL_MESSAGE( "Wrong editable", string( "true"), editable );
 }
 
+void JsonTest::parseDeepNestingTest( )
+{
+    // A response with thousands of [[[[...]]]] would blow the stack inside
+    // boost::property_tree::json_parser::read_json. Json::parse must
+    // refuse to feed it to read_json, falling back to the malformed-input
+    // path.
+    string deep( 5000, '[' );
+    deep.append( 5000, ']' );
+    Json json = Json::parse( deep );
+    CPPUNIT_ASSERT_EQUAL( deep, json.toString( ) );
+}
+
 void JsonTest::parseTypeTest( )
 {
     Json json = parseFile( DATA_DIR "/gdrive/jsontest-good.json" );

qa/libcmis/test-soap.cxx

@@ -69,12 +69,14 @@ class SoapTest : public CppUnit::TestFixture
         void parseResponseTest( );
         void parseResponseXmlTest( );
         void parseResponseFaultTest( );
+        void parseResponseNoNamespaceTest( );
 
         // RelatedMultipart tests
 
         void serializeMultipartSimpleTest( );
         void serializeMultipartComplexTest( );
         void parseMultipartTest( );
+        void parseMultipartEmptyFieldsTest( );
         void getStreamFromNodeXopTest( );
         void getStreamFromNodeBase64Test( );
 
@@ -91,10 +93,12 @@ class SoapTest : public CppUnit::TestFixture
         CPPUNIT_TEST( parseResponseTest );
         CPPUNIT_TEST( parseResponseXmlTest );
         CPPUNIT_TEST( parseResponseFaultTest );
+        CPPUNIT_TEST( parseResponseNoNamespaceTest );
 
         CPPUNIT_TEST( serializeMultipartSimpleTest );
         CPPUNIT_TEST( serializeMultipartComplexTest );
         CPPUNIT_TEST( parseMultipartTest );
+        CPPUNIT_TEST( parseMultipartEmptyFieldsTest );
         CPPUNIT_TEST( getStreamFromNodeXopTest );
         CPPUNIT_TEST( getStreamFromNodeBase64Test );
 
@@ -316,6 +320,21 @@ void SoapTest::parseResponseFaultTest( )
     }
 }
 
+void SoapTest::parseResponseNoNamespaceTest( )
+{
+    SoapResponseFactory factory;
+    factory.setMapping( getTestMapping() );
+    factory.setNamespaces( getTestNamespaces( ) );
+    factory.setDetailMapping( getTestDetailMapping( ) );
+
+    string xml = "<S:Envelope xmlns:S=\"http://schemas.xmlsoap.org/soap/envelope/\"><S:Body>"
+                 "<evil/>"
+                 "</S:Body></S:Envelope>";
+
+    vector< SoapResponsePtr > actual = factory.parseResponse( xml );
+    CPPUNIT_ASSERT_EQUAL_MESSAGE( "Wrong number of responses", size_t( 0 ), actual.size( ) );
+}
+
 void SoapTest::serializeMultipartSimpleTest( )
 {
     string partName = "data";
@@ -443,6 +462,32 @@ void SoapTest::parseMultipartTest( )
     CPPUNIT_ASSERT_EQUAL_MESSAGE( "Wrong part2 part content", part2Content, actualPart2->getContent( ) );
 }
 
+void SoapTest::parseMultipartEmptyFieldsTest( )
+{
+    // Body, contentType params and Content-ID can all be empty in a
+    // bogus response.
+    string boundary = "abc";
+
+    {
+        // Empty start= and an empty body.
+        string contentType = "multipart/related; start=\"\"; boundary=\"" + boundary + "\"";
+        RelatedMultipart multipart( "", contentType );
+        CPPUNIT_ASSERT_EQUAL( string( ), multipart.getStartId( ) );
+    }
+
+    {
+        // A part with an empty Content-Id and an empty body.
+        string body = "\r\n--" + boundary + "\r\n" +
+                      "Content-Id: \r\n" +
+                      "Content-Type: text/plain\r\n" +
+                      "\r\n" +
+                      "\r\n--" + boundary + "--\r\n";
+        string contentType = "multipart/related; boundary=\"" + boundary + "\"";
+        RelatedMultipart multipart( body, contentType );
+        CPPUNIT_ASSERT_EQUAL( boundary, multipart.getBoundary( ) );
+    }
+}
+
 void SoapTest::getStreamFromNodeXopTest( )
 {
     // Create the test multipart

src/cmis-client.cxx

@@ -481,7 +481,15 @@ void CmisClient::execute( )
                     streamId = m_vm["stream-id"].as<string>();
 
                 boost::shared_ptr< istream > in = document->getContentStream( streamId );
-                ofstream out( document->getContentFilename().c_str() );
+                string filename = document->getContentFilename();
+                if ( filename.empty() ||
+                     filename == "." || filename == ".." ||
+                     filename.find( '/' ) != string::npos ||
+                     filename.find( '\\' ) != string::npos )
+                {
+                    throw CommandException( "Refusing server-supplied filename: " + filename );
+                }
+                ofstream out( filename.c_str() );
                 out << in->rdbuf();
                 out.close();
             }

src/libcmis-c/document.cxx

@@ -148,15 +148,14 @@ void libcmis_document_getContentStream(
                 boost::shared_ptr< istream > stream = doc->getContentStream( );
 
                 stream->seekg( 0 );
-                int bufSize = 2048;
-                char* buf = new char[ bufSize ];
+                size_t bufSize = 2048;
+                std::vector< char > buf( bufSize );
                 while ( !stream->eof( ) )
                 {
-                    stream->read( buf, bufSize );
+                    stream->read( buf.data(), bufSize );
                     size_t read = stream->gcount( );
-                    writeFn( ( const void * )buf, size_t( 1 ), read, userData );
+                    writeFn( ( const void * )buf.data(), size_t( 1 ), read, userData );
                 }
-                delete[] buf;
             }
         }
         catch ( const libcmis::Exception& e )
@@ -203,14 +202,13 @@ void libcmis_document_setContentStream(
             boost::shared_ptr< std::ostream > stream( new stringstream( ) );
 
             size_t bufSize = 2048;
-            char* buf = new char[ bufSize ];
+            std::vector< char > buf( bufSize );
             size_t read = 0;
             do
             {
-                read = readFn( ( void * )buf, size_t( 1 ), bufSize, userData );
-                stream->write( buf, read );
+                read = readFn( ( void * )buf.data(), size_t( 1 ), bufSize, userData );
+                stream->write( buf.data(), read );
             } while ( read == bufSize );
-            delete[] buf;
 
             DocumentPtr doc = dynamic_pointer_cast< libcmis::Document >( document->handle );
             if ( doc )
@@ -358,14 +356,13 @@ libcmis_DocumentPtr libcmis_document_checkIn(
                 boost::shared_ptr< std::ostream > stream( new stringstream( ) );
 
                 size_t bufSize = 2048;
-                char * buf = new char[ bufSize ];
+                std::vector< char > buf( bufSize );
                 size_t read = 0;
                 do
                 {
-                    read = readFn( ( void * )buf, size_t( 1 ), bufSize, userData );
-                    stream->write( buf, read );
+                    read = readFn( ( void * )buf.data(), size_t( 1 ), bufSize, userData );
+                    stream->write( buf.data(), read );
                 } while ( read == bufSize );
-                delete[] buf;
 
                 // Create the property map
                 PropertyPtrMap propertiesMap;

src/libcmis-c/folder.cxx

@@ -276,14 +276,13 @@ libcmis_DocumentPtr libcmis_folder_createDocument(
                 boost::shared_ptr< std::ostream > stream( new stringstream( ) );
 
                 size_t bufSize = 2048;
-                char* buf = new char[ bufSize ];
+                std::vector< char > buf( bufSize );
                 size_t read = 0;
                 do
                 {
-                    read = readFn( ( void * )buf, size_t( 1 ), bufSize, userData );
-                    stream->write( buf, read );
+                    read = readFn( ( void * )buf.data(), size_t( 1 ), bufSize, userData );
+                    stream->write( buf.data(), read );
                 } while ( read == bufSize );
-                delete[] buf;
 
                 // Create the property map
                 PropertyPtrMap propertiesMap;

src/libcmis/allowable-actions.cxx

@@ -54,8 +54,11 @@ namespace libcmis
         try
         {
             xmlChar* content = xmlNodeGetContent( node );
-            m_enabled = parseBool( string( ( char* )content ) );
-            xmlFree( content );
+            if ( content )
+            {
+                m_enabled = parseBool( string( ( char* )content ) );
+                xmlFree( content );
+            }
         }
         catch ( const Exception& )
         {