Bug #601
closedlibhtp hook_run_all not thread safe
Description
Suricata assures thread safety on the flow level for HTTP tracking. Part of the flow is (in case of HTTP) libhtp's htp_connp_t state.
At startup the libhtp glue layer, app-layer-htp initializes as many htp_cfg_t instances as there are libhtp server configurations in the yaml.
At HTTP session start, we look up the proper htp_cfg_t based on the server ip and pass it to htp_connp_create.
A ptr to the relevant htp_cfg_t is part of the htp_connp_t
The htp_cfg_t contains "hooks". The are registered based on yaml config at init time.
The hooks have lists of type list_t.
The list is run with a built in iterator.
The iterator is reset at the start of each "hook_run_all".
Since multiple flows share the same htp_cfg_t flow A can reset the iterator while flow B is using it.
The flow lock has no effect as flows share the htp_cfg_t.
This has been observed in real traffic. hook_response_body_data was run on the same data multiple times, leading to corrupt extracted files.
Issue has been comfirmed by libhtp upstream.
Files
Updated by Victor Julien over 11 years ago
- File 0001-libhtp-don-t-use-internal-iterator.patch 0001-libhtp-don-t-use-internal-iterator.patch added
Attached patch is proposed solution.
Updated by Victor Julien over 11 years ago
- Status changed from Assigned to Closed
- % Done changed from 0 to 100
Pushed:
commit 2cdbdab38c878da01518ae932a55984538786e81
Author: Victor Julien <victor@inliniac.net>
Date: Fri Oct 12 16:40:43 2012 +0200
libhtp: don't use internal iterator
It violates thread safety. #601.
Suricata assures thread safety on the flow level for HTTP tracking. Part of the flow is (in case of HTTP) libhtp's htp_connp_t state. At startup the libhtp glue layer, app-layer-htp initializes as many htp_cfg_t instances as there ar
The hooks have lists of type list_t. The list is run with a built in iterator. The iterator is reset at the start of each "hook_run_all". Since multiple flows share the same htp_cfg_t flow A can reset the iterator while flow B is usi
This has been observed in real traffic. hook_response_body_data was run on the same data multiple times, leading to corrupt extracted files.
Applied it to master as well.