From: Oleksandr Stepanov -X (ostepano - SOFTSERVE INC at Cisco) Date: Thu, 13 Nov 2025 11:54:34 +0000 (+0000) Subject: Pull request #4977: appid: ftp parsing bounds check X-Git-Tag: 3.10.0.0~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3f055d2d858f564629e2a3da500a8af042029d3;p=thirdparty%2Fsnort3.git Pull request #4977: appid: ftp parsing bounds check Merge in SNORT/snort3 from ~OSTEPANO/snort3:ftp_overflow to master Squashed commit of the following: commit 725fa41fe90f105ce77b67ba120b8bd3778018c3 Author: Oleksandr Stepanov Date: Wed Nov 5 06:48:08 2025 -0500 appid: ftp parsing bounds check --- diff --git a/src/network_inspectors/appid/service_plugins/service_ftp.cc b/src/network_inspectors/appid/service_plugins/service_ftp.cc index 0dd9b6e4d..bd34492d4 100644 --- a/src/network_inspectors/appid/service_plugins/service_ftp.cc +++ b/src/network_inspectors/appid/service_plugins/service_ftp.cc @@ -317,7 +317,7 @@ static int CheckVendorVersion(const uint8_t* data, uint16_t init_offset, static FtpEolReturn ftp_parse_response(const uint8_t* data, uint16_t& offset, uint16_t size, ServiceFTPData& fd, FTPReplyState rstate) { - for (; offset < size; ++offset) + while (offset < size) { if (data[offset] == 0x0D) { @@ -338,6 +338,13 @@ static FtpEolReturn ftp_parse_response(const uint8_t* data, uint16_t& offset, fd.rstate = rstate; return FTP_FOUND_EOL; } + + if (offset == UINT16_MAX) + { + return FTP_NOT_FOUND_EOL; + } + + ++offset; } return FTP_NOT_FOUND_EOL; } @@ -389,13 +396,18 @@ static int ftp_validate_reply(const uint8_t* data, uint16_t& offset, uint16_t si FtpEolReturn parse_ret; FTPReplyState tmp_state; - for (; offset < size; ++offset) + while (offset < size) { /* Trim any blank lines (be a little tolerant) */ - for (; offset < size; ++offset) + while (offset < size) { if (data[offset] != 0x0D and data[offset] != 0x0A) break; + if (offset == UINT16_MAX) + { + return 0; + } + ++offset; } switch (fd.rstate) @@ -535,6 +547,8 @@ static int ftp_validate_reply(const uint8_t* data, uint16_t& offset, uint16_t si parse_ret = ftp_parse_response(data, offset, size, fd, FTP_REPLY_LONG); if (parse_ret == FTP_INCORRECT_EOL) return -1; + if (offset == UINT16_MAX) + return 0; if (++offset >= size) { offset = size; @@ -605,7 +619,7 @@ static int ftp_validate_reply(const uint8_t* data, uint16_t& offset, uint16_t si } if (fd.rstate == FTP_REPLY_BEGIN) { - for (; offset < size; ++offset) + while (offset < size) { if (data[offset] == 0x0D) { @@ -616,9 +630,20 @@ static int ftp_validate_reply(const uint8_t* data, uint16_t& offset, uint16_t si } else if (!isspace(data[offset])) break; + + if (offset == UINT16_MAX) + { + return 0; + } + ++offset; } return fd.code; } + if (offset == UINT16_MAX) + { + return 0; + } + ++offset; } return 0; } diff --git a/src/network_inspectors/appid/service_plugins/test/CMakeLists.txt b/src/network_inspectors/appid/service_plugins/test/CMakeLists.txt index 6c78f2a3b..8fe78eee0 100644 --- a/src/network_inspectors/appid/service_plugins/test/CMakeLists.txt +++ b/src/network_inspectors/appid/service_plugins/test/CMakeLists.txt @@ -7,3 +7,4 @@ add_cpputest( service_snmp_test ) add_cpputest( service_rtmp_test ) add_cpputest( service_netbios_test ) add_cpputest( service_nntp_test ) +add_cpputest( service_ftp_test ) diff --git a/src/network_inspectors/appid/service_plugins/test/service_ftp_test.cc b/src/network_inspectors/appid/service_plugins/test/service_ftp_test.cc new file mode 100644 index 000000000..8d7777418 --- /dev/null +++ b/src/network_inspectors/appid/service_plugins/test/service_ftp_test.cc @@ -0,0 +1,113 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2022-2025 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- +// +// service_ftp_test.cc author Oleksandr Stepanov + +#define FTP_UNIT_TEST + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include "../service_ftp.h" +#include "../service_ftp.cc" +#include "service_plugin_mock.h" + +#include +#include +#include + +const uint8_t* service_strstr(const uint8_t* p, unsigned, + const uint8_t*, unsigned) +{ + return nullptr; +} + +AppIdModule::AppIdModule() + : Module("a", "b") { } +AppIdModule::~AppIdModule() = default; + +AppIdConfig test_app_config; + +static ServiceFTPData mock_service_data; +static ServiceDiscovery mock_service_discovery; +static FtpServiceDetector test_detector(&mock_service_discovery); + +AppIdInspector::AppIdInspector(AppIdModule&) : config(&test_app_config), ctxt(test_app_config) { } + +AppIdFlowData* AppIdDetector::data_get(const AppIdSession&) +{ + return &mock_service_data; +} + +TEST_GROUP(ftp_parsing_tests) +{ + void setup() override + { + + } + void teardown() override + { + + } +}; + +TEST(ftp_parsing_tests, ftp_parse_invalid_offset_reply) +{ + uint8_t* data = new uint8_t[65535]; + uint16_t offset = 65530; + ServiceFTPData fd; + fd.rstate = FTP_REPLY_MULTI; + + memset(data, 0, 65535); + data[65534] = 0x0D; + data[65533] = 0x0D; + + auto res = ftp_validate_reply(data, offset, 65535, fd); + CHECK_EQUAL(0, res); + delete[] data; +} + +TEST(ftp_parsing_tests, ftp_parse_invalid_offset_response) +{ + uint8_t* data = new uint8_t[65535]; + uint16_t offset = 65533; + ServiceFTPData fd; + fd.rstate = FTP_REPLY_MULTI; + + memset(data, 0, 65535); + data[65534] = 0x0D; + data[65533] = 0x0D; + data[65532] = 0x0D; + + auto res = ftp_parse_response(data, offset, 65535, fd, FTP_REPLY_MULTI); + CHECK_EQUAL(FTP_PARTIAL_EOL, res); + + offset = 65534; + memset(data, 0, 65535); + res = ftp_parse_response(data, offset, 65535, fd, FTP_REPLY_MULTI); + CHECK_EQUAL(FTP_NOT_FOUND_EOL, res); + + delete[] data; +} + +int main(int argc, char** argv) +{ + int return_value = CommandLineTestRunner::RunAllTests(argc, argv); + return return_value; +} \ No newline at end of file diff --git a/src/network_inspectors/appid/service_plugins/test/service_plugin_mock.h b/src/network_inspectors/appid/service_plugins/test/service_plugin_mock.h index 5fae71be9..55ce81082 100644 --- a/src/network_inspectors/appid/service_plugins/test/service_plugin_mock.h +++ b/src/network_inspectors/appid/service_plugins/test/service_plugin_mock.h @@ -98,7 +98,9 @@ void ClientDiscovery::reload() {} int AppIdDetector::initialize(AppIdInspector&){return 0;} int AppIdDetector::data_add(AppIdSession&, AppIdFlowData*){return 0;} +#ifndef FTP_UNIT_TEST AppIdFlowData* AppIdDetector::data_get(const AppIdSession&) {return nullptr;} +#endif void AppIdDetector::add_user(AppIdSession&, const char*, AppId, bool, AppidChangeBits&){} void AppIdDetector::add_payload(AppIdSession&, AppId){} void AppIdDetector::add_app(const snort::Packet&, AppIdSession&, AppidSessionDirection, AppId, AppId, const char*, AppidChangeBits&){} @@ -234,6 +236,26 @@ AppIdHttpSession* AppIdSession::get_http_session(uint32_t) const { return nullpt AppIdHttpSession* AppIdSession::create_http_session(int64_t stream_id) { return nullptr; } void AppIdHttpSession::set_field(HttpFieldIds id, const std::string* str, AppidChangeBits&) { } +void AppIdModule::reset_stats() {} +void AppIdModule::sum_stats(bool) { } +AppIdInspector::~AppIdInspector() = default; +SfIpRet snort::SfIp::set(void const*, int) { return SFIP_SUCCESS; } +SfIpRet snort::SfIp::set(void const*) { return SFIP_SUCCESS; } +SfIpRet snort::SfIp::pton(const int, const char* ) { return SFIP_SUCCESS; } +void AppIdInspector::eval(snort::Packet*) { } +void AppIdInspector::show(const snort::SnortConfig*) const { } +void AppIdInspector::tinit() { } +void AppIdInspector::tterm() { } +void AppIdInspector::tear_down(snort::SnortConfig*) { } +snort::ProfileStats* AppIdModule::get_profile( + unsigned, const char*&, const char*&) const +{ + return nullptr; +} +bool AppIdInspector::configure(snort::SnortConfig*) { return true; } +PegCount snort::Module::get_global_count(char const*) const { return 0; } +snort::Module::Module(char const*, char const*) {} + void snort::DataBus::publish(unsigned, unsigned, snort::DataEvent&, snort::Flow*) { } void snort::DataBus::publish(unsigned, unsigned, const uint8_t*, unsigned, snort::Flow*) { } void snort::DataBus::publish(unsigned, unsigned, snort::Packet*, snort::Flow*) { }