summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorEdwin Mons <edwin.mons@isode.com>2018-11-09 10:04:04 (GMT)
committerEdwin Mons <edwin.mons@isode.com>2018-11-14 14:18:08 (GMT)
commitccad2debbf8d7322c9d2b517763d7b8e3902a828 (patch)
tree50054ea69dcf21179920ffdde5790908e48848d8
parentc7ad127218e3901e0006e75aa7e1399b449a845e (diff)
downloadswift-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.h12
-rw-r--r--Swiften/FileTransfer/ByteArrayReadBytestream.cpp22
-rw-r--r--Swiften/FileTransfer/IBBSendSession.cpp4
-rw-r--r--Swiften/FileTransfer/SOCKS5BytestreamServerSession.cpp11
-rw-r--r--Swiften/FileTransfer/UnitTest/IBBSendSessionTest.cpp4
-rw-r--r--Swiften/Parser/PayloadParsers/IBBParser.cpp4
-rw-r--r--Swiften/QA/FileTransferTest/FileTransferTest.cpp9
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
31void ByteArrayReadBytestream::addData(const std::vector<unsigned char>& moreData) { 37void 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
41void IBBSendSession::start() { 41void 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