From ccad2debbf8d7322c9d2b517763d7b8e3902a828 Mon Sep 17 00:00:00 2001
From: Edwin Mons <edwin.mons@isode.com>
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<IBB>(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<unsigned char> 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<ByteArray> ByteArrayReadBytestream::read(size_t size) {
if (position + readSize > data.size()) {
readSize = data.size() - position;
}
- std::shared_ptr<ByteArray> result = std::make_shared<ByteArray>(
- data.begin() + boost::numeric_cast<long long>(position),
- data.begin() + boost::numeric_cast<long long>(position) + boost::numeric_cast<long long>(readSize));
-
- onRead(*result);
- position += readSize;
- return result;
+ try {
+ std::shared_ptr<ByteArray> result = std::make_shared<ByteArray>(
+ data.begin() + boost::numeric_cast<long long>(position),
+ data.begin() + boost::numeric_cast<long long>(position) + boost::numeric_cast<long long>(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<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 @@
/*
- * 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<int>(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<unsigned char>(requestID.size()));
+ try {
+ result.push_back(boost::numeric_cast<unsigned char>(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<IBB>(0, JID("foo@bar.com/baz"), IQ::Set));
IBB::ref ibb = stanzaChannel->sentStanzas[0]->getPayload<IBB>();
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<int>(attributes.getAttribute("block-size")));
+ getPayloadInternal()->setBlockSize(boost::lexical_cast<unsigned int>(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 <boost/algorithm/string.hpp>
#include <boost/filesystem.hpp>
-#include <boost/numeric/conversion/cast.hpp>
#include <Swiften/Base/Debug.h>
@@ -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<size_t>(randGen.generateRandomInteger(1024 * 10));
+ size_t size = 1024 + static_cast<size_t>(randGen.generateRandomInteger(1024 * 10));
sendData_.resize(size);
for (unsigned char& n : sendData_) {
- n = boost::numeric_cast<unsigned char>(randGen.generateRandomInteger(255));
+ n = static_cast<unsigned char>(randGen.generateRandomInteger(255));
}
std::ofstream outfile(sendFilePath_.native().c_str(), std::ios::out | std::ios::binary);
- outfile.write(reinterpret_cast<char *>(&sendData_[0]), boost::numeric_cast<ptrdiff_t>(sendData_.size()));
+ outfile.write(reinterpret_cast<char *>(&sendData_[0]), static_cast<ptrdiff_t>(sendData_.size()));
outfile.close();
}
--
cgit v0.10.2-6-g49f6