Suricata

Bug #9162

Updated by Jason Ish 7 days ago

h2. ## Summary 

 In @src/source-pcap-file-directory-helper.c@, @PcapDirectoryDispatchForTimeRange()@ `src/source-pcap-file-directory-helper.c`, 
 `PcapDirectoryDispatchForTimeRange()` does: 

 <pre><code class="c"> ```c 
 pv->current_file = pftv; 

 status = PcapFileDispatch(pftv); 

 CleanupPcapFileFileVars(pftv); 

 if (status == TM_ECODE_FAILED) { 
     CleanupPendingFile(current_file); 
     SCReturnInt(status); 
 } 
 ... 
 pv->current_file = NULL; 
 </code></pre> ``` 

 @pv->current_file@ `pv->current_file` is only cleared at the end of the success path. When @PcapFileDispatch()@ 
 `PcapFileDispatch()` returns @TM_ECODE_FAILED@ `TM_ECODE_FAILED` we return early after @CleanupPcapFileFileVars()@ 
 `CleanupPcapFileFileVars()` has released @pftv@ `pftv` (or deferred the release until 
 the last in-flight packet is gone), and @pv->current_file@ `pv->current_file` still points at it. 
 On thread teardown @CleanupPcapFileDirectoryVars()@ `CleanupPcapFileDirectoryVars()` sees a non-NULL @current_file@ 
 `current_file` and calls @CleanupPcapFileFileVars()@ `CleanupPcapFileFileVars()` on it again. 

 h2. ## Confirmed present in 

 * @oisf/main@ at @7ad9874c050188cc3cf4c7eafdf90a277f534fb7@ - `oisf/main` @ `7ad9874c050188cc3cf4c7eafdf90a277f534fb7` 

 h2. ## Impact 

 Whenever @PcapFileDispatch()@ `PcapFileDispatch()` returns @TM_ECODE_FAILED@ `TM_ECODE_FAILED` in directory mode the 
 pointer is left dangling. That happens when the packet pipeline fails: @TmThreadsSlotProcessPkt()@ 
 `TmThreadsSlotProcessPkt()` returns an error in @PcapFileCallbackLoop()@ `PcapFileCallbackLoop()` and @cb_result@ 
 `cb_result` is set to @TM_ECODE_FAILED@. `TM_ECODE_FAILED`. The pointer is then freed or 
 dereferenced a second time at shutdown (or when the thread is torn down for any 
 other reason): use-after-free / double free, which crashes the process or 
 corrupts the heap if the memory was reused. This is local only, it needs pcap 
 directory mode (e.g. @--pcap-file-continuous@) `--pcap-file-continuous`) and is not reachable from the 
 network. 

 h2. ## Preconditions 

 pcap directory mode, and the packet pipeline reports a failure while a file 
 from the watched directory is being processed. 

 h2. ## Proposed fix 

 Clear the pointer right after the file vars are cleaned up, so neither the early 
 return nor the success path leaves it dangling. The existing assignment at the 
 end of the success path becomes redundant and moves up: 

 <pre><code class="c"> ```c 
 CleanupPcapFileFileVars(pftv); 
 pv->current_file = NULL; 
 </code></pre> ``` 

 h2. ## Validation status 

 This issue was identified through source-level review, not by running a sensor, 
 so we don't have a pcap or a crash to attach. We also could not find an input 
 file that triggers the path; the trigger is a failure in the packet pipeline, 
 not a specific malformed pcap. We compared the success and failure paths of @PcapDirectoryDispatchForTimeRange()@ 
 `PcapDirectoryDispatchForTimeRange()` and the teardown in @CleanupPcapFileDirectoryVars()@. 
 `CleanupPcapFileDirectoryVars()`. 

 h2. ## Attachments 

 * @0002-pcap-directory-clear-current-file-after-cleanup.patch@ - `0002-pcap-directory-clear-current-file-after-cleanup.patch` (issue 2) 
 * @regression_test_snippet_pcap_directory.c@ - `regression_test_snippet_pcap_directory.c` (issue 2) 

 h2. ## Suggested severity 

 Our view, we'll defer to your assessment: issue 2 looks LOW (local only, needs a 
 pipeline failure in directory mode, no network vector). 

 h2. ## Credit 

 Please credit Alexander Stadnikov and CC Luukas Larinkoski. 

 No other individuals need to be named. 

Back