From ccad2debbf8d7322c9d2b517763d7b8e3902a828 Mon Sep 17 00:00:00 2001 From: Edwin Mons Date: Fri, 9 Nov 2018 11:04:04 +0100 Subject: Address bad_numeric_casts for filetransfers The filetransfer blockSize is now an unsigned integer, as 0 could be used to denote an invalid block size as well (and indeed, already indicated that better than -1 did). All use of numeric_cast in filetransfer code has been fixed to deal with the possibility of thrown exceptions. Test-Information: Unit tests pass on macOS and Debian Change-Id: I1833d553bae071238be20ebc386ef602effb78b0 diff --git a/Swiften/Elements/IBB.h b/Swiften/Elements/IBB.h index bd0b661..6ebe66e 100644 --- a/Swiften/Elements/IBB.h +++ b/Swiften/Elements/IBB.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -28,10 +28,10 @@ namespace Swift { MessageStanza }; - IBB(Action action = Open, const std::string& streamID = "") : action(action), streamID(streamID), stanzaType(IQStanza), blockSize(-1), sequenceNumber(-1) { + IBB(Action action = Open, const std::string& streamID = "") : action(action), streamID(streamID), stanzaType(IQStanza), blockSize(0), sequenceNumber(-1) { } - static IBB::ref createIBBOpen(const std::string& streamID, int blockSize) { + static IBB::ref createIBBOpen(const std::string& streamID, unsigned int blockSize) { IBB::ref result = std::make_shared(Open, streamID); result->setBlockSize(blockSize); return result; @@ -80,11 +80,11 @@ namespace Swift { this->data = data; } - int getBlockSize() const { + unsigned int getBlockSize() const { return blockSize; } - void setBlockSize(int blockSize) { + void setBlockSize(unsigned int blockSize) { this->blockSize = blockSize; } @@ -101,7 +101,7 @@ namespace Swift { std::string streamID; std::vector data; StanzaType stanzaType; - int blockSize; + unsigned int blockSize; int sequenceNumber; }; } diff --git a/Swiften/FileTransfer/ByteArrayReadBytestream.cpp b/Swiften/FileTransfer/ByteArrayReadBytestream.cpp index cd9fa4a..3fdff27 100644 --- a/Swiften/FileTransfer/ByteArrayReadBytestream.cpp +++ b/Swiften/FileTransfer/ByteArrayReadBytestream.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -19,13 +19,19 @@ std::shared_ptr ByteArrayReadBytestream::read(size_t size) { if (position + readSize > data.size()) { readSize = data.size() - position; } - std::shared_ptr result = std::make_shared( - data.begin() + boost::numeric_cast(position), - data.begin() + boost::numeric_cast(position) + boost::numeric_cast(readSize)); - - onRead(*result); - position += readSize; - return result; + try { + std::shared_ptr result = std::make_shared( + data.begin() + boost::numeric_cast(position), + data.begin() + boost::numeric_cast(position) + boost::numeric_cast(readSize)); + onRead(*result); + position += readSize; + return result; + } + catch (const boost::numeric::bad_numeric_cast&) { + // If we cannot cast to long long, we probably ran out of memory long ago + assert(false); + return {}; + } } void ByteArrayReadBytestream::addData(const std::vector& moreData) { diff --git a/Swiften/FileTransfer/IBBSendSession.cpp b/Swiften/FileTransfer/IBBSendSession.cpp index e51c91c..258412b 100644 --- a/Swiften/FileTransfer/IBBSendSession.cpp +++ b/Swiften/FileTransfer/IBBSendSession.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -40,7 +40,7 @@ IBBSendSession::~IBBSendSession() { void IBBSendSession::start() { IBBRequest::ref request = IBBRequest::create( - from, to, IBB::createIBBOpen(id, boost::numeric_cast(blockSize)), router); + from, to, IBB::createIBBOpen(id, blockSize), router); request->onResponse.connect(boost::bind(&IBBSendSession::handleIBBResponse, this, _1, _2)); active = true; request->send(); diff --git a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp index bc4e8e4..0fd40bf 100644 --- a/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp +++ b/Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -138,7 +138,14 @@ void SOCKS5BytestreamServerSession::process() { SafeByteArray result = createSafeByteArray("\x05", 1); result.push_back(hasBytestream ? 0x0 : 0x4); append(result, createByteArray("\x00\x03", 2)); - result.push_back(boost::numeric_cast(requestID.size())); + try { + result.push_back(boost::numeric_cast(requestID.size())); + } + catch (const boost::numeric::bad_numeric_cast& e) { + SWIFT_LOG(warning) << "SOCKS5 request ID is too long (" << requestID.size() << "): " << e.what() << std::endl; + finish(); + return; + } append(result, concat(requestID, createByteArray("\x00\x00", 2))); if (!hasBytestream) { SWIFT_LOG(debug) << "Readstream or Wrtiestream with ID " << streamID << " not found!" << std::endl; diff --git a/Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp b/Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp index f9057f8..2399cbe 100644 --- a/Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp +++ b/Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -58,7 +58,7 @@ class IBBSendSessionTest : public CppUnit::TestFixture { CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex(0, JID("foo@bar.com/baz"), IQ::Set)); IBB::ref ibb = stanzaChannel->sentStanzas[0]->getPayload(); CPPUNIT_ASSERT_EQUAL(IBB::Open, ibb->getAction()); - CPPUNIT_ASSERT_EQUAL(1234, ibb->getBlockSize()); + CPPUNIT_ASSERT_EQUAL(1234u, ibb->getBlockSize()); CPPUNIT_ASSERT_EQUAL(std::string("myid"), ibb->getStreamID()); } diff --git a/Swiften/Parser/PayloadParsers/IBBParser.cpp b/Swiften/Parser/PayloadParsers/IBBParser.cpp index 9b6babc..1ba44e1 100644 --- a/Swiften/Parser/PayloadParsers/IBBParser.cpp +++ b/Swiften/Parser/PayloadParsers/IBBParser.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2010-2016 Isode Limited. + * Copyright (c) 2010-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -39,7 +39,7 @@ void IBBParser::handleStartElement(const std::string& element, const std::string getPayloadInternal()->setStanzaType(IBB::IQStanza); } try { - getPayloadInternal()->setBlockSize(boost::lexical_cast(attributes.getAttribute("block-size"))); + getPayloadInternal()->setBlockSize(boost::lexical_cast(attributes.getAttribute("block-size"))); } catch (boost::bad_lexical_cast&) { } diff --git a/Swiften/QA/FileTransferTest/FileTransferTest.cpp b/Swiften/QA/FileTransferTest/FileTransferTest.cpp index ebdb36a..7d69277 100644 --- a/Swiften/QA/FileTransferTest/FileTransferTest.cpp +++ b/Swiften/QA/FileTransferTest/FileTransferTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2016 Isode Limited. + * Copyright (c) 2014-2018 Isode Limited. * All rights reserved. * See the COPYING file for more information. */ @@ -8,7 +8,6 @@ #include #include -#include #include @@ -78,14 +77,14 @@ class FileTransferTest { sendFilePath_ = boost::filesystem::unique_path("ft_send_%%%%%%%%%%%%%%%%.bin"); receiveFilePath_ = boost::filesystem::unique_path("ft_receive_%%%%%%%%%%%%%%%%.bin"); - size_t size = 1024 + boost::numeric_cast(randGen.generateRandomInteger(1024 * 10)); + size_t size = 1024 + static_cast(randGen.generateRandomInteger(1024 * 10)); sendData_.resize(size); for (unsigned char& n : sendData_) { - n = boost::numeric_cast(randGen.generateRandomInteger(255)); + n = static_cast(randGen.generateRandomInteger(255)); } std::ofstream outfile(sendFilePath_.native().c_str(), std::ios::out | std::ios::binary); - outfile.write(reinterpret_cast(&sendData_[0]), boost::numeric_cast(sendData_.size())); + outfile.write(reinterpret_cast(&sendData_[0]), static_cast(sendData_.size())); outfile.close(); } -- cgit v0.10.2-6-g49f6