From: Ashutosh Gupta (ashugup3) Date: Fri, 14 Feb 2025 08:11:36 +0000 (+0000) Subject: Pull request #4601: file_api: Fix for file capture issue X-Git-Tag: 3.7.1.0~24 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d017be4e548e04afeb0a74f4626f4383f7b37a3c;p=thirdparty%2Fsnort3.git Pull request #4601: file_api: Fix for file capture issue Merge in SNORT/snort3 from ~ASHUGUP3/snort3:bug_CSCwn57820 to master Squashed commit of the following: commit ea8bad098f8578f924bae4f7957c631b31a5717f Author: ashutosh Date: Tue Feb 4 17:01:05 2025 +0530 file_api: Setting current file data inside mutex with file data received before accessing it --- diff --git a/src/file_api/file_cache.cc b/src/file_api/file_cache.cc index dcb4943ed..b27e400f7 100644 --- a/src/file_api/file_cache.cc +++ b/src/file_api/file_cache.cc @@ -282,7 +282,7 @@ FileContext* FileCache::get_file(Flow* flow, uint64_t file_id, bool to_create, b } FileVerdict FileCache::check_verdict(Packet* p, FileInfo* file, - FilePolicyBase* policy) + FilePolicyBase* policy, const uint8_t* current_data, uint32_t current_data_len) { assert(file); @@ -301,6 +301,7 @@ FileVerdict FileCache::check_verdict(Packet* p, FileInfo* file, if ( file->get_file_sig_sha256() and verdict <= FILE_VERDICT_LOG ) { file->user_file_data_mutex.lock(); + file->set_capture_file_data(current_data, current_data_len); verdict = policy->signature_lookup(p, file); file->user_file_data_mutex.unlock(); } @@ -524,7 +525,7 @@ bool FileCache::apply_verdict(Packet* p, FileContext* file_ctx, FileVerdict verd } FileVerdict FileCache::cached_verdict_lookup(Packet* p, FileInfo* file, - FilePolicyBase* policy) + FilePolicyBase* policy, const uint8_t* current_data, uint32_t current_data_len) { Flow* flow = p->flow; FileVerdict verdict = FILE_VERDICT_UNKNOWN; @@ -543,7 +544,7 @@ FileVerdict FileCache::cached_verdict_lookup(Packet* p, FileInfo* file, if (file_found) { /*Query the file policy in case verdict has been changed*/ - verdict = check_verdict(p, file_found, policy); + verdict = check_verdict(p, file_found, policy, current_data, current_data_len); FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_DEBUG_LEVEL, p, "cached_verdict_lookup:Verdict received from cached_verdict_lookup %d\n", verdict); apply_verdict(p, file_found, verdict, true, policy); diff --git a/src/file_api/file_cache.h b/src/file_api/file_cache.h index bd1dcf533..78d7bd958 100644 --- a/src/file_api/file_cache.h +++ b/src/file_api/file_cache.h @@ -62,7 +62,7 @@ PADDING_GUARD_END snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create, bool using_cache_entry); FileVerdict cached_verdict_lookup(snort::Packet*, snort::FileInfo*, - snort::FilePolicyBase*); + snort::FilePolicyBase*,const uint8_t* current_data, uint32_t current_data_len); bool apply_verdict(snort::Packet*, snort::FileContext*, FileVerdict, bool resume, snort::FilePolicyBase*); @@ -72,7 +72,7 @@ private: snort::FileContext* find_add(const FileHashKey&, int64_t); snort::FileContext* get_file(snort::Flow*, uint64_t file_id, bool to_create, int64_t timeout, bool using_cache_entry, bool &cache_full); - FileVerdict check_verdict(snort::Packet*, snort::FileInfo*, snort::FilePolicyBase*); + FileVerdict check_verdict(snort::Packet*, snort::FileInfo*, snort::FilePolicyBase*,const uint8_t* current_data, uint32_t current_data_len); int store_verdict(snort::Flow*, snort::FileInfo*, int64_t timeout, bool &cache_full); /* The hash table of expected files */ diff --git a/src/file_api/file_capture.h b/src/file_api/file_capture.h index aed12d0e1..10e4786e6 100644 --- a/src/file_api/file_capture.h +++ b/src/file_api/file_capture.h @@ -96,6 +96,7 @@ public: int64_t get_max_file_capture_size() { return capture_max_size; } int64_t get_file_capture_size() { return capture_size; } void get_file_reset() { current_block = head; } + void set_data(const uint8_t* file_data, const uint32_t size) {current_data = file_data; current_data_len = size;} private: diff --git a/src/file_api/file_flows.cc b/src/file_api/file_flows.cc index 26fad6a30..aff3f9cf9 100644 --- a/src/file_api/file_flows.cc +++ b/src/file_api/file_flows.cc @@ -382,7 +382,7 @@ bool FileFlows::file_process(Packet* p, uint64_t file_id, const uint8_t* file_da if ((context->is_cacheable() and not is_new_context) or context->is_partial_download()) // If partial header was seen - check file_cache { FileVerdict verdict = FileService::get_file_cache()->cached_verdict_lookup(p, context, - file_policy); + file_policy, file_data, data_size); if (verdict != FILE_VERDICT_UNKNOWN and verdict != FILE_VERDICT_PENDING) { context->processing_complete = true; diff --git a/src/file_api/file_lib.cc b/src/file_api/file_lib.cc index c95503996..192bce037 100644 --- a/src/file_api/file_lib.cc +++ b/src/file_api/file_lib.cc @@ -366,6 +366,12 @@ void FileInfo::set_file_data(UserFileDataBase* fd) user_file_data = fd; } +void FileInfo::set_capture_file_data(const uint8_t* file_data, uint32_t size) +{ + if (file_capture) + file_capture->set_data(file_data, size); +} + UserFileDataBase* FileInfo::get_file_data() const { return user_file_data; @@ -598,7 +604,7 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, return false; } - if (cacheable and (FileService::get_file_cache()->cached_verdict_lookup(p, this, policy) != + if (cacheable and (FileService::get_file_cache()->cached_verdict_lookup(p, this, policy, file_data, data_size) != FILE_VERDICT_UNKNOWN)) { FILE_DEBUG(file_trace, DEFAULT_TRACE_OPTION_ID, TRACE_INFO_LEVEL, @@ -687,7 +693,9 @@ bool FileContext::process(Packet* p, const uint8_t* file_data, int data_size, /*Fails to capture, when out of memory or size limit, need lookup*/ if (is_file_capture_enabled()) { + user_file_data_mutex.lock(); process_file_capture(file_data, data_size, position); + user_file_data_mutex.unlock(); } finish_signature_lookup(p, ( file_state.sig_state != FILE_SIG_FLUSH ), policy); diff --git a/src/file_api/file_lib.h b/src/file_api/file_lib.h index 2b612356c..063a99656 100644 --- a/src/file_api/file_lib.h +++ b/src/file_api/file_lib.h @@ -95,6 +95,7 @@ public: UserFileDataBase* get_file_data() const; void copy(const FileInfo& other, bool clear_data = true); void reset(); + void set_capture_file_data(const uint8_t* file_data, uint32_t size); // Preserve the file in memory until it is released // The file reserved will be returned and it will be detached from file context/session FileCaptureState reserve_file(FileCapture*& dest);