diff options
| author | Edwin Mons <edwin.mons@isode.com> | 2018-11-09 10:04:04 (GMT) |
|---|---|---|
| committer | Edwin Mons <edwin.mons@isode.com> | 2018-11-14 14:18:08 (GMT) |
| commit | ccad2debbf8d7322c9d2b517763d7b8e3902a828 (patch) | |
| tree | 50054ea69dcf21179920ffdde5790908e48848d8 | |
| parent | c7ad127218e3901e0006e75aa7e1399b449a845e (diff) | |
| download | swift-ccad2debbf8d7322c9d2b517763d7b8e3902a828.zip swift-ccad2debbf8d7322c9d2b517763d7b8e3902a828.tar.bz2 | |
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
| -rw-r--r-- | Swiften/Elements/IBB.h | 12 | ||||
| -rw-r--r-- | Swiften/FileTransfer/ByteArrayReadBytestream.cpp | 22 | ||||
| -rw-r--r-- | Swiften/FileTransfer/IBBSendSession.cpp | 4 | ||||
| -rw-r--r-- | Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp | 11 | ||||
| -rw-r--r-- | Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp | 4 | ||||
| -rw-r--r-- | Swiften/Parser/PayloadParsers/IBBParser.cpp | 4 | ||||
| -rw-r--r-- | Swiften/QA/FileTransferTest/FileTransferTest.cpp | 9 |
7 files changed, 39 insertions, 27 deletions
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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -28,10 +28,10 @@ namespace Swift { | |||
| 28 | MessageStanza | 28 | MessageStanza |
| 29 | }; | 29 | }; |
| 30 | 30 | ||
| 31 | IBB(Action action = Open, const std::string& streamID = "") : action(action), streamID(streamID), stanzaType(IQStanza), blockSize(-1), sequenceNumber(-1) { | 31 | IBB(Action action = Open, const std::string& streamID = "") : action(action), streamID(streamID), stanzaType(IQStanza), blockSize(0), sequenceNumber(-1) { |
| 32 | } | 32 | } |
| 33 | 33 | ||
| 34 | static IBB::ref createIBBOpen(const std::string& streamID, int blockSize) { | 34 | static IBB::ref createIBBOpen(const std::string& streamID, unsigned int blockSize) { |
| 35 | IBB::ref result = std::make_shared<IBB>(Open, streamID); | 35 | IBB::ref result = std::make_shared<IBB>(Open, streamID); |
| 36 | result->setBlockSize(blockSize); | 36 | result->setBlockSize(blockSize); |
| 37 | return result; | 37 | return result; |
| @@ -80,11 +80,11 @@ namespace Swift { | |||
| 80 | this->data = data; | 80 | this->data = data; |
| 81 | } | 81 | } |
| 82 | 82 | ||
| 83 | int getBlockSize() const { | 83 | unsigned int getBlockSize() const { |
| 84 | return blockSize; | 84 | return blockSize; |
| 85 | } | 85 | } |
| 86 | 86 | ||
| 87 | void setBlockSize(int blockSize) { | 87 | void setBlockSize(unsigned int blockSize) { |
| 88 | this->blockSize = blockSize; | 88 | this->blockSize = blockSize; |
| 89 | } | 89 | } |
| 90 | 90 | ||
| @@ -101,7 +101,7 @@ namespace Swift { | |||
| 101 | std::string streamID; | 101 | std::string streamID; |
| 102 | std::vector<unsigned char> data; | 102 | std::vector<unsigned char> data; |
| 103 | StanzaType stanzaType; | 103 | StanzaType stanzaType; |
| 104 | int blockSize; | 104 | unsigned int blockSize; |
| 105 | int sequenceNumber; | 105 | int sequenceNumber; |
| 106 | }; | 106 | }; |
| 107 | } | 107 | } |
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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -19,13 +19,19 @@ std::shared_ptr<ByteArray> ByteArrayReadBytestream::read(size_t size) { | |||
| 19 | if (position + readSize > data.size()) { | 19 | if (position + readSize > data.size()) { |
| 20 | readSize = data.size() - position; | 20 | readSize = data.size() - position; |
| 21 | } | 21 | } |
| 22 | std::shared_ptr<ByteArray> result = std::make_shared<ByteArray>( | 22 | try { |
| 23 | data.begin() + boost::numeric_cast<long long>(position), | 23 | std::shared_ptr<ByteArray> result = std::make_shared<ByteArray>( |
| 24 | data.begin() + boost::numeric_cast<long long>(position) + boost::numeric_cast<long long>(readSize)); | 24 | data.begin() + boost::numeric_cast<long long>(position), |
| 25 | 25 | data.begin() + boost::numeric_cast<long long>(position) + boost::numeric_cast<long long>(readSize)); | |
| 26 | onRead(*result); | 26 | onRead(*result); |
| 27 | position += readSize; | 27 | position += readSize; |
| 28 | return result; | 28 | return result; |
| 29 | } | ||
| 30 | catch (const boost::numeric::bad_numeric_cast&) { | ||
| 31 | // If we cannot cast to long long, we probably ran out of memory long ago | ||
| 32 | assert(false); | ||
| 33 | return {}; | ||
| 34 | } | ||
| 29 | } | 35 | } |
| 30 | 36 | ||
| 31 | void ByteArrayReadBytestream::addData(const std::vector<unsigned char>& moreData) { | 37 | void ByteArrayReadBytestream::addData(const std::vector<unsigned char>& 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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -40,7 +40,7 @@ IBBSendSession::~IBBSendSession() { | |||
| 40 | 40 | ||
| 41 | void IBBSendSession::start() { | 41 | void IBBSendSession::start() { |
| 42 | IBBRequest::ref request = IBBRequest::create( | 42 | IBBRequest::ref request = IBBRequest::create( |
| 43 | from, to, IBB::createIBBOpen(id, boost::numeric_cast<int>(blockSize)), router); | 43 | from, to, IBB::createIBBOpen(id, blockSize), router); |
| 44 | request->onResponse.connect(boost::bind(&IBBSendSession::handleIBBResponse, this, _1, _2)); | 44 | request->onResponse.connect(boost::bind(&IBBSendSession::handleIBBResponse, this, _1, _2)); |
| 45 | active = true; | 45 | active = true; |
| 46 | request->send(); | 46 | 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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -138,7 +138,14 @@ void SOCKS5BytestreamServerSession::process() { | |||
| 138 | SafeByteArray result = createSafeByteArray("\x05", 1); | 138 | SafeByteArray result = createSafeByteArray("\x05", 1); |
| 139 | result.push_back(hasBytestream ? 0x0 : 0x4); | 139 | result.push_back(hasBytestream ? 0x0 : 0x4); |
| 140 | append(result, createByteArray("\x00\x03", 2)); | 140 | append(result, createByteArray("\x00\x03", 2)); |
| 141 | result.push_back(boost::numeric_cast<unsigned char>(requestID.size())); | 141 | try { |
| 142 | result.push_back(boost::numeric_cast<unsigned char>(requestID.size())); | ||
| 143 | } | ||
| 144 | catch (const boost::numeric::bad_numeric_cast& e) { | ||
| 145 | SWIFT_LOG(warning) << "SOCKS5 request ID is too long (" << requestID.size() << "): " << e.what() << std::endl; | ||
| 146 | finish(); | ||
| 147 | return; | ||
| 148 | } | ||
| 142 | append(result, concat(requestID, createByteArray("\x00\x00", 2))); | 149 | append(result, concat(requestID, createByteArray("\x00\x00", 2))); |
| 143 | if (!hasBytestream) { | 150 | if (!hasBytestream) { |
| 144 | SWIFT_LOG(debug) << "Readstream or Wrtiestream with ID " << streamID << " not found!" << std::endl; | 151 | 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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -58,7 +58,7 @@ class IBBSendSessionTest : public CppUnit::TestFixture { | |||
| 58 | CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<IBB>(0, JID("foo@bar.com/baz"), IQ::Set)); | 58 | CPPUNIT_ASSERT(stanzaChannel->isRequestAtIndex<IBB>(0, JID("foo@bar.com/baz"), IQ::Set)); |
| 59 | IBB::ref ibb = stanzaChannel->sentStanzas[0]->getPayload<IBB>(); | 59 | IBB::ref ibb = stanzaChannel->sentStanzas[0]->getPayload<IBB>(); |
| 60 | CPPUNIT_ASSERT_EQUAL(IBB::Open, ibb->getAction()); | 60 | CPPUNIT_ASSERT_EQUAL(IBB::Open, ibb->getAction()); |
| 61 | CPPUNIT_ASSERT_EQUAL(1234, ibb->getBlockSize()); | 61 | CPPUNIT_ASSERT_EQUAL(1234u, ibb->getBlockSize()); |
| 62 | CPPUNIT_ASSERT_EQUAL(std::string("myid"), ibb->getStreamID()); | 62 | CPPUNIT_ASSERT_EQUAL(std::string("myid"), ibb->getStreamID()); |
| 63 | } | 63 | } |
| 64 | 64 | ||
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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2010-2016 Isode Limited. | 2 | * Copyright (c) 2010-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -39,7 +39,7 @@ void IBBParser::handleStartElement(const std::string& element, const std::string | |||
| 39 | getPayloadInternal()->setStanzaType(IBB::IQStanza); | 39 | getPayloadInternal()->setStanzaType(IBB::IQStanza); |
| 40 | } | 40 | } |
| 41 | try { | 41 | try { |
| 42 | getPayloadInternal()->setBlockSize(boost::lexical_cast<int>(attributes.getAttribute("block-size"))); | 42 | getPayloadInternal()->setBlockSize(boost::lexical_cast<unsigned int>(attributes.getAttribute("block-size"))); |
| 43 | } | 43 | } |
| 44 | catch (boost::bad_lexical_cast&) { | 44 | catch (boost::bad_lexical_cast&) { |
| 45 | } | 45 | } |
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 @@ | |||
| 1 | /* | 1 | /* |
| 2 | * Copyright (c) 2014-2016 Isode Limited. | 2 | * Copyright (c) 2014-2018 Isode Limited. |
| 3 | * All rights reserved. | 3 | * All rights reserved. |
| 4 | * See the COPYING file for more information. | 4 | * See the COPYING file for more information. |
| 5 | */ | 5 | */ |
| @@ -8,7 +8,6 @@ | |||
| 8 | 8 | ||
| 9 | #include <boost/algorithm/string.hpp> | 9 | #include <boost/algorithm/string.hpp> |
| 10 | #include <boost/filesystem.hpp> | 10 | #include <boost/filesystem.hpp> |
| 11 | #include <boost/numeric/conversion/cast.hpp> | ||
| 12 | 11 | ||
| 13 | 12 | ||
| 14 | #include <Swiften/Base/Debug.h> | 13 | #include <Swiften/Base/Debug.h> |
| @@ -78,14 +77,14 @@ class FileTransferTest { | |||
| 78 | sendFilePath_ = boost::filesystem::unique_path("ft_send_%%%%%%%%%%%%%%%%.bin"); | 77 | sendFilePath_ = boost::filesystem::unique_path("ft_send_%%%%%%%%%%%%%%%%.bin"); |
| 79 | receiveFilePath_ = boost::filesystem::unique_path("ft_receive_%%%%%%%%%%%%%%%%.bin"); | 78 | receiveFilePath_ = boost::filesystem::unique_path("ft_receive_%%%%%%%%%%%%%%%%.bin"); |
| 80 | 79 | ||
| 81 | size_t size = 1024 + boost::numeric_cast<size_t>(randGen.generateRandomInteger(1024 * 10)); | 80 | size_t size = 1024 + static_cast<size_t>(randGen.generateRandomInteger(1024 * 10)); |
| 82 | sendData_.resize(size); | 81 | sendData_.resize(size); |
| 83 | for (unsigned char& n : sendData_) { | 82 | for (unsigned char& n : sendData_) { |
| 84 | n = boost::numeric_cast<unsigned char>(randGen.generateRandomInteger(255)); | 83 | n = static_cast<unsigned char>(randGen.generateRandomInteger(255)); |
| 85 | } | 84 | } |
| 86 | 85 | ||
| 87 | std::ofstream outfile(sendFilePath_.native().c_str(), std::ios::out | std::ios::binary); | 86 | std::ofstream outfile(sendFilePath_.native().c_str(), std::ios::out | std::ios::binary); |
| 88 | outfile.write(reinterpret_cast<char *>(&sendData_[0]), boost::numeric_cast<ptrdiff_t>(sendData_.size())); | 87 | outfile.write(reinterpret_cast<char *>(&sendData_[0]), static_cast<ptrdiff_t>(sendData_.size())); |
| 89 | outfile.close(); | 88 | outfile.close(); |
| 90 | } | 89 | } |
| 91 | 90 | ||
Swift