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
VJ Updated by Victor Julien over 13 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.
VJ Updated by Victor Julien over 13 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.