From 25d7806f6d0a0d826dfc29bd6d18b26f1e53241c Mon Sep 17 00:00:00 2001 From: BodgeMaster <> Date: Mon, 17 Oct 2022 16:01:57 +0200 Subject: [PATCH 1/3] lib/nbt: Fix memory leak in case of parsing error --- src/lib/nbt.cpp | 53 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/src/lib/nbt.cpp b/src/lib/nbt.cpp index 5264ecf..aa8cfc5 100644 --- a/src/lib/nbt.cpp +++ b/src/lib/nbt.cpp @@ -1176,6 +1176,7 @@ namespace NBT { // FIXME: memory leak when returning errors ErrorOr> deserializeRawListContents(uint8_t data[], uint64_t dataSize, uint64_t initialPosition, uint64_t* processedDataSize) { std::vector contents; + ErrorOr> returnValue; // get contained data length by reading it manually because // the function that does it normally can't deal with // headerless tags @@ -1183,6 +1184,8 @@ namespace NBT { // add one byte to position to skip the type byte ErrorOr elementCount = Helper::readInt32(data, dataSize, initialPosition+1); if (elementCount.isError) { + // this is before the creation of any pointers so we can just return + // without using the returnError label at the end of this function return ErrorOr>(true, elementCount.errorCode); } @@ -1202,14 +1205,10 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt8(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int8("", nextInt.value)); - // The below code would produce a warning on GCC and Clang - // about the computed value not being used. While this does - // apply inside this function, it is ultimately not true - // as the pointer is used both inside and outside of the - // function. *processedDataSize += 1; } break; @@ -1218,7 +1217,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt16(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int16("", nextInt.value)); *processedDataSize += 2; @@ -1229,7 +1229,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt32(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int32("", nextInt.value)); *processedDataSize += 4; @@ -1240,7 +1241,8 @@ namespace NBT { for (int32_t i=0; i nextFloat = Helper::readFloat(data, dataSize, initialPosition+*processedDataSize); if (nextFloat.isError) { - return ErrorOr>(true, nextFloat.errorCode); + returnValue = ErrorOr>(true, nextFloat.errorCode); + goto returnError; } contents.push_back(new Tag::Float("", nextFloat.value)); *processedDataSize += 4; @@ -1251,7 +1253,8 @@ namespace NBT { for (int32_t i=0; i nextInt = Helper::readInt64(data, dataSize, initialPosition+*processedDataSize); if (nextInt.isError) { - return ErrorOr>(true, nextInt.errorCode); + returnValue = ErrorOr>(true, nextInt.errorCode); + goto returnError; } contents.push_back(new Tag::Int64("", nextInt.value)); *processedDataSize += 8; @@ -1262,7 +1265,8 @@ namespace NBT { for (int32_t i=0; i nextDouble = Helper::readDouble(data, dataSize, initialPosition+*processedDataSize); if (nextDouble.isError) { - return ErrorOr>(true, nextDouble.errorCode); + returnValue = ErrorOr>(true, nextDouble.errorCode); + goto returnError; } contents.push_back(new Tag::Double("", nextDouble.value)); *processedDataSize += 8; @@ -1273,7 +1277,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt8Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int8Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size(); @@ -1284,7 +1289,8 @@ namespace NBT { for (int32_t i=0; i nextString = Helper::readString(data, dataSize, initialPosition+*processedDataSize); if (nextString.isError) { - return ErrorOr>(true, nextString.errorCode); + returnValue = ErrorOr>(true, nextString.errorCode); + goto returnError; } contents.push_back(new Tag::String("", nextString.value)); // this cannot be an error because it just got read @@ -1301,7 +1307,8 @@ namespace NBT { ErrorOr> nextListContents = deserializeRawListContents(data, dataSize, initialPosition+*processedDataSize, containedDataSize); if (nextListContents.isError) { delete containedDataSize; - return ErrorOr>(true, nextListContents.errorCode); + returnValue = ErrorOr>(true, nextListContents.errorCode); + goto returnError; } contents.push_back(new Tag::List("", nextListContents.value)); *processedDataSize += *containedDataSize; @@ -1316,7 +1323,8 @@ namespace NBT { ErrorOr> nextCompoundData = deserialize(data, dataSize, initialPosition+*processedDataSize, containedDataSize); if (nextCompoundData.isError) { delete containedDataSize; - return ErrorOr>(true, nextCompoundData.errorCode); + returnValue = ErrorOr>(true, nextCompoundData.errorCode); + goto returnError; } contents.push_back(new Tag::Compound("", nextCompoundData.value)); *processedDataSize += *containedDataSize; @@ -1328,7 +1336,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt32Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int32Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size() * 4; @@ -1339,7 +1348,8 @@ namespace NBT { for (int32_t i=0; i> nextArray = Helper::readInt64Array(data, dataSize, initialPosition+*processedDataSize); if (nextArray.isError) { - return ErrorOr>(true, nextArray.errorCode); + returnValue = ErrorOr>(true, nextArray.errorCode); + goto returnError; } contents.push_back(new Tag::Int64Array("", nextArray.value)); *processedDataSize += (uint64_t) nextArray.value.size() * 8; @@ -1347,9 +1357,16 @@ namespace NBT { break; } default: - return ErrorOr>(true, ErrorCodes::INVALID_TYPE); + returnValue = ErrorOr>(true, ErrorCodes::INVALID_TYPE); + goto returnError; } return ErrorOr>(contents); + + returnError: + for (uint64_t i=0; i Date: Tue, 18 Oct 2022 05:56:32 +0200 Subject: [PATCH 2/3] tools/hexnet: Starting over --- src/tools/hexnet.cpp | 306 +------------------------------------------ 1 file changed, 1 insertion(+), 305 deletions(-) diff --git a/src/tools/hexnet.cpp b/src/tools/hexnet.cpp index 5efb5f2..6728026 100644 --- a/src/tools/hexnet.cpp +++ b/src/tools/hexnet.cpp @@ -34,309 +34,5 @@ #define EXIT_USAGE 2 #define EXIT_UNIMPLEMENTED 3 -bool ipv4 = true; -bool ipv6 = true; -bool tcp = true; -bool udp = true; -bool listenMode = false; -int64_t mtu = 1500; -std::string host; -in_port_t port; -sockpp::tcp_socket* tcpSocket; -sockpp::tcp6_socket* tcp6Socket; -sockpp::udp_socket* udpSocket; -sockpp::tcp_acceptor tcpAcceptor; -sockpp::tcp6_acceptor tcp6Acceptor; -std::mutex tcpSocketMutex; -std::mutex tcp6SocketMutex; -std::mutex udpSocketMutex; -std::mutex udp6SocketMutex; -std::mutex consoleMutex; -// used for coordinated graceful exit across threads -bool exitProgram = false; - -void signalHandler(int signal) { - exitProgram = true; - // if still waiting for incoming connection, stop waiting - tcpAcceptor.shutdown(); - tcp6Acceptor.shutdown(); - // tell sockpp to close TCP socket if open because it blocks when trying - // to read and there is no data - - if (tcpSocket != nullptr && *tcpSocket) { - // Intentionally not using the mutex here - std::cout << "test\n"; - tcpSocket->shutdown(SHUT_RD); - } - - if (tcp6Socket != nullptr && *tcp6Socket) { - // Intentionally not using the mutex here - tcp6Socket->shutdown(SHUT_RD); - } - //TODO: figure out if - and how - this applies to UDP - - // Priority is to finish up all unfinished business that can be finished up. - // If something has the console mutex locked, that should not prevent - // other threads from winding down. This is why logging happens last. - consoleMutex.lock(); - std::cerr << "Received signal " << signal << ", shutting down." << std::endl; - consoleMutex.unlock(); - std::exit(signal); -} - -void readFromConsole(){ - while(!exitProgram){ - char * test = new char[420]; - consoleMutex.lock(); - std::cout << "> "; - std::cin.read(test, 1); - consoleMutex.unlock(); - } -} - -void readFromTCPSocket(sockpp::tcp_socket* socket, int64_t mtu) { - ssize_t numBytes; - uint8_t buffer[mtu]; - tcpSocketMutex.lock(); - while (!exitProgram && (numBytes = socket->read(buffer, sizeof(buffer))) > 0) { - tcpSocketMutex.unlock(); - consoleMutex.lock(); - std::cout << "\n"; - for (ssize_t i=0; iread(buffer, sizeof(buffer))) > 0) { - tcp6SocketMutex.unlock(); - consoleMutex.lock(); - for (ssize_t i=0; irecv_from(buffer, sizeof(buffer), &srcAddr) > 0)){ - udpSocketMutex.unlock(); - consoleMutex.lock(); - for(ssize_t i=0; i flags; - flags.push_back(CLI::Flag('4', "ipv4", "use IPv4, defaults to both when -4 and -6 are omitted, otherwise uses what is specified")); - flags.push_back(CLI::Flag('6', "ipv6", "use IPv6, defaults to both when -4 and -6 are omitted, otherwise uses what is specified")); - flags.push_back(CLI::Flag('t', "tcp", "use TCP, defaults to both when -t and -u are omitted, otherwise uses what is specified")); - flags.push_back(CLI::Flag('u', "udp", "use UDP, defaults to both when -t and -u are omitted, otherwise uses what is specified")); - flags.push_back(CLI::Flag('n', "no-color", "disable coloring the output (intended for terminals that don't work well with color escape sequences)")); - flags.push_back(CLI::Flag('e', "echo-back", "echo input back to stdout")); - flags.push_back(CLI::Flag('h', "help", "print this information and exit")); - flags.push_back(CLI::Flag('l', "license", "print license information and exit")); - - std::vector options; - options.push_back(CLI::Option('c', "connect", "HOST", "connect to HOST, listen for incoming connections if omitted")); - options.push_back(CLI::Option('m', "mtu-optimize", "MTU", "Optimize for a specific maximum transfer unit by reading MTU bytes at a time.")); - options.push_back(CLI::Option( - 'p', "print-prefixes", "TCPin:UDPin:TCPout:UDPout", - "override default prefixes for output (defaults to spaces + coloring the output or \"t:u:T:U\" in no-color mode)" - )); - options.push_back(CLI::Option( - 'i', "input-prefixes", "TCP:UDP", - "override default prefixes for input (defaults to \"t:u\")" - )); - - std::vector arguments; - arguments.push_back(CLI::Argument("PORT", "the port to use")); - - CLI::ArgumentsParser cliParser = CLI::ArgumentsParser(argc, argv, flags, options, arguments, "Arbitrary tcp/udp connections in hex format."); - - if (cliParser.getFlag("help").value){ - std::cout << cliParser.getUsage() << std::endl; - return EXIT_SUCCESS; - } - if (cliParser.getFlag("license").value){ - std::cout - << "Copyright 2022, FOSS-VG Developers and Contributers\n" - << "\n" - << "Hexnet is part of the FOSS-VG development tool suite.\n" - << "\n" - << "This program is free software: you can redistribute it and/or modify it\n" - << "under the terms of the GNU Affero General Public License as published\n" - << "by the Free Software Foundation, version 3.\n" - << "\n" - << "This program is distributed in the hope that it will be useful,\n" - << "but WITHOUT ANY WARRANTY; without even the implied\n" - << "warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n" - << "See the GNU Affero General Public License for more details.\n" - << "\n" - << "You should have received a copy of the GNU Affero General Public License\n" - << "version 3 along with this program.\n" - << "If not, see https://www.gnu.org/licenses/agpl-3.0.en.html" - << std::endl; - return EXIT_SUCCESS; - } - - if (cliParser.wrongUsage) { - std::cout << cliParser.getUsage() << std::endl; - return EXIT_USAGE; - } - if (cliParser.getFlag("ipv4").value || cliParser.getFlag("ipv6").value) { - ipv4 = cliParser.getFlag("ipv4").value; - ipv6 = cliParser.getFlag("ipv6").value; - } - if (cliParser.getFlag("tcp").value || cliParser.getFlag("udp").value) { - tcp = cliParser.getFlag("tcp").value; - udp = cliParser.getFlag("udp").value; - } - if (cliParser.getOption('c').errorCode == ErrorCodes::NOT_PRESENT) { - listenMode = true; - } - if (cliParser.getOption("mtu-optimize").errorCode == ErrorCodes::SUCCESS) { - mtu = std::stol(cliParser.getOption("mtu-optimize").value); - } - host = cliParser.getOption("connect").value; - //FIXME: use a function that returns a fixed-width data type instead, - // ensure that the given value is a valid port - port = (in_port_t) std::stoi(cliParser.getArgument(0).value); - - if (listenMode) { - if (udp && ipv4) { - std::cerr << "Listening on port " << port << "." << std::endl; - - if(!udpSocket->bind(sockpp::inet_address("localhost", port))){ - std::cerr << "Error while binding UDP socket: " << udpSocket->last_error_str() << std::endl; - return EXIT_RUNTIME; - } - - std::thread threadReadFromUDP = std::thread(readFromUDPSocket, udpSocket, mtu); - threadReadFromUDP.join(); - - delete udpSocket; - return EXIT_SUCCESS; - } - - if (ipv6) { - std::cerr << "Listening on port " << port << "." << std::endl; - - tcp6Acceptor = sockpp::tcp6_acceptor(port); - - if(!tcp6Acceptor){ - std::cerr << "Error while creating TCP6 acceptor: " << tcp6Acceptor.last_error_str() << std::endl; - return EXIT_RUNTIME; - } - - sockpp::inet6_address peer; - tcp6Socket = new sockpp::tcp6_socket(); - *tcp6Socket = tcp6Acceptor.accept(&peer); - - std::cerr << "Incoming connection from " << peer << std::endl; - - if (!(*tcp6Socket)) { - std::cerr << "Error on incoming connection: " << tcp6Acceptor.last_error_str() << std::endl; - delete tcp6Socket; - return EXIT_RUNTIME; - } - - std::thread threadReadFromTCP6 = std::thread(readFromTCP6Socket, tcp6Socket, mtu); - threadReadFromTCP6.join(); - - delete tcp6Socket; - return EXIT_SUCCESS; - } - - if(ipv4){ - std::cerr << "Listening on port " << port << "." << std::endl; - - - tcpAcceptor = sockpp::tcp_acceptor(port); - - if (!tcpAcceptor) { - std::cerr << "Error while creating TCP acceptor: " << tcpAcceptor.last_error_str() << std::endl; - return EXIT_RUNTIME; - } - - sockpp::inet_address peer; - tcpSocket = new sockpp::tcp_socket(); - *tcpSocket = tcpAcceptor.accept(&peer); - - std::cerr << "Incoming connection from " << peer << std::endl; - - if (!(*tcpSocket)) { - std::cerr << "Error on incoming connection: " << tcpAcceptor.last_error_str() << std::endl; - delete tcpSocket; - return EXIT_RUNTIME; - } - - std::thread threadReadFromTCP = std::thread(readFromTCPSocket, tcpSocket, mtu); - std::thread threadReadFromConsole = std::thread(readFromConsole); - - threadReadFromConsole.join(); - threadReadFromTCP.join(); - - delete tcpSocket; - return EXIT_SUCCESS; - } - - } else { - std::cerr << "Client mode is not implemented yet." << std::endl; - return EXIT_UNIMPLEMENTED; - } +int main(int argc, char* argv[]) { } From ee9b5d4f673e2a144b17b292eed2b9aea525d7a2 Mon Sep 17 00:00:00 2001 From: BodgeMaster <> Date: Wed, 19 Oct 2022 15:24:42 +0200 Subject: [PATCH 3/3] tools/hexnet: Add command line parser back Reimplementation has started. --- scripts/build.sh | 2 +- src/tools/hexnet.cpp | 96 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/scripts/build.sh b/scripts/build.sh index 9a936d2..0d6d4b1 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -62,7 +62,7 @@ COMPILE_COMMANDS=( "$CXX_WITH_FLAGS src/tools/dumpnbt.cpp -I./include -Lbin/lib -l:nbt.so -l:javacompat.so -l:cli.so -o bin/tools/dumpnbt" "$CXX_WITH_FLAGS src/tools/arraydump.cpp -I./include -Lbin/lib -l:file.so -l:cli.so -o bin/tools/arraydump" "$CXX_WITH_FLAGS src/tools/baseconvert.cpp -I./include -Lbin/lib -l:cli.so -o bin/tools/baseconvert" - #"$CXX_WITH_FLAGS src/tools/hexnet.cpp -I./include -Lbin/lib -l:cli.so -l:libsockpp.so -o bin/tools/hexnet" + "$CXX_WITH_FLAGS src/tools/hexnet.cpp -I./include -Lbin/lib -l:cli.so -l:libsockpp.so -o bin/tools/hexnet" ) for command in ${!COMPILE_COMMANDS[@]}; do echo "${COMPILE_COMMANDS[command]}" diff --git a/src/tools/hexnet.cpp b/src/tools/hexnet.cpp index 6728026..5fd088c 100644 --- a/src/tools/hexnet.cpp +++ b/src/tools/hexnet.cpp @@ -1,5 +1,8 @@ //Copyright 2022, FOSS-VG Developers and Contributers // +// Author(s): +// BodgeMaster, Shwoomple +// //This program is free software: you can redistribute it and/or modify it //under the terms of the GNU Affero General Public License as published //by the Free Software Foundation, version 3. @@ -13,26 +16,93 @@ //version 3 along with this program. //If not, see https://www.gnu.org/licenses/agpl-3.0.en.html -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include +#include +#include +#include +#include -#include "../lib/error.hpp" #include "../lib/cli.hpp" +#include "../lib/error.hpp" #define EXIT_SUCCESS 0 #define EXIT_RUNTIME 1 #define EXIT_USAGE 2 #define EXIT_UNIMPLEMENTED 3 -int main(int argc, char* argv[]) { +std::mutex mutexStdin; +std::mutex mutexStdout; +std::mutex mutexNetIncoming; +std::mutex mutexNetOutgoing; + +bool ipv4 = false; +bool ipv6 = false; +bool tcp = false; +bool udp = false; + +void signalHandler(int signal) { + // shut down gracefully + std::cerr << "Received signal " << signal << ", shutting down." << std::endl; + std::exit(signal); +} + +int main(int argc, char* argv[]) { + std::signal(SIGINT, signalHandler); + std::signal(SIGTERM, signalHandler); + + std::vector flags; + flags.push_back(CLI::Flag('4', "ipv4", "use IPv4, either this or IPv6 has to be specified")); + flags.push_back(CLI::Flag('6', "ipv6", "use IPv6, either this or IPv4 has to be specified")); + flags.push_back(CLI::Flag('t', "tcp", "use TCP, either this or UDP has to be specified")); + flags.push_back(CLI::Flag('u', "udp", "use UDP, either this or TCP has to be specified")); + flags.push_back(CLI::Flag('h', "help", "print this information and exit")); + flags.push_back(CLI::Flag('l', "license", "print license information and exit")); + + std::vector options; + options.push_back(CLI::Option('c', "connect", "HOST", "make an outgoing connection to HOST instead of listening for an incoming connection")); + + std::vector arguments; + arguments.push_back(CLI::Argument("PORT", "the port to lsiten on (or connect to)")); + + CLI::ArgumentsParser cliParser = CLI::ArgumentsParser(argc, argv, flags, options, arguments, "Arbitrary TCP/UDP connections in hex format"); + + if (cliParser.getFlag("help").value) { + std::cout << cliParser.getUsage() << std::endl; + return EXIT_SUCCESS; + } + if (cliParser.getFlag("license").value) { + std::cout + << "Copyright 2022, FOSS-VG Developers and Contributers\n" + << "\n" + << "Hexnet is part of the FOSS-VG development tool suite.\n" + << "\n" + << "This program is free software: you can redistribute it and/or modify it\n" + << "under the terms of the GNU Affero General Public License as published\n" + << "by the Free Software Foundation, version 3.\n" + << "\n" + << "This program is distributed in the hope that it will be useful,\n" + << "but WITHOUT ANY WARRANTY; without even the implied\n" + << "warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n" + << "See the GNU Affero General Public License for more details.\n" + << "\n" + << "You should have received a copy of the GNU Affero General Public License\n" + << "version 3 along with this program.\n" + << "If not, see https://www.gnu.org/licenses/agpl-3.0.en.html" + << std::endl; + return EXIT_SUCCESS; + } + if (cliParser.wrongUsage) { + std::cout << cliParser.getUsage() << std::endl; + return EXIT_USAGE; + } + + ipv4 = cliParser.getFlag("ipv4").value; + ipv6 = cliParser.getFlag("ipv6").value; + tcp = cliParser.getFlag("tcp").value; + udp = cliParser.getFlag("udp").value; + + if (!(ipv4 || ipv6) || !(tcp || udp)) { + std::cout << cliParser.getUsage() << std::endl; + return EXIT_USAGE; + } }