BUG/MAJOR: filters: Always keep all offsets up to date during data filtering
When at least one data filter is registered on a channel, the offsets of all
filters must be kept up to date. For data filters but also for others. It is
safer to do it in that way. Indirectly, this patch fixes 2 hidden bugs
revealed by the commit 22fca1f2c ("BUG/MEDIUM: filters: Forward all filtered
data at the end of http filtering").
The first one, the worst of both, happens at the end of http filtering when
at least one data filtered is registered on the channel. We call the
http_end() callback function on the filters, when defined, to finish the
http filtering. But it is performed for all filters. Before the commit
22fca1f2c, the only risk was to call the http_end() callback function
unexpectedly on a filter. Now, we may have an overflow on the offset
variable, used at the end to forward all filtered data. Of course, from the
moment we forward an arbitrary huge amount of data, all kinds of bad things
may happen. So offset computation is performed for all filters and
http_end() callback function is called only for data filters.
The other one happens when a data filter alter the data of a channel, it
must update the offsets of all previous filters. But the offset of non-data
filters must be up to date, otherwise, here too we may have an integer
overflow.
Another way to fix these bugs is to always ignore non-data filters from the
offsets computation. But this patch is safer and probably easier to
maintain.
This patch must be backported in all versions where the above commit is. So
as far as 2.0.
(cherry picked from commit 401e6dbff3ee0b1932f6a16e3f280246752a7edf)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/filters.c b/src/filters.c
index 326b062..ca72d75 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -555,6 +555,12 @@
unsigned long long flt_off = FLT_OFF(filter, msg->chn);
offset = flt_off - *strm_off;
+ /* Call http_end for data filters only. But the filter offset is
+ * still valid for all filters
+ . */
+ if (!IS_DATA_FILTER(filter, msg->chn))
+ continue;
+
if (FLT_OPS(filter)->http_end) {
DBG_TRACE_DEVEL(FLT_ID(filter), STRM_EV_HTTP_ANA|STRM_EV_FLT_ANA, s);
ret = FLT_OPS(filter)->http_end(s, filter, msg);
@@ -629,13 +635,18 @@
ret = data = len - out;
DBG_TRACE_ENTER(STRM_EV_STRM_ANA|STRM_EV_HTTP_ANA|STRM_EV_FLT_ANA, s, s->txn, msg);
list_for_each_entry(filter, &strm_flt(s)->filters, list) {
- /* Call "data" filters only */
- if (!IS_DATA_FILTER(filter, msg->chn))
+ unsigned long long *flt_off = &FLT_OFF(filter, msg->chn);
+ unsigned int offset = *flt_off - *strm_off;
+
+ /* Call http_payload for filters only. Forward all data for
+ * others and update the filter offset
+ */
+ if (!IS_DATA_FILTER(filter, msg->chn)) {
+ *flt_off += data - offset;
continue;
- if (FLT_OPS(filter)->http_payload) {
- unsigned long long *flt_off = &FLT_OFF(filter, msg->chn);
- unsigned int offset = *flt_off - *strm_off;
+ }
+ if (FLT_OPS(filter)->http_payload) {
DBG_TRACE_DEVEL(FLT_ID(filter), STRM_EV_HTTP_ANA|STRM_EV_FLT_ANA, s);
ret = FLT_OPS(filter)->http_payload(s, filter, msg, out + offset, data - offset);
if (ret < 0)
@@ -793,10 +804,8 @@
size_t data = http_get_hdrs_size(htxbuf(&chn->buf));
struct filter *f;
- list_for_each_entry(f, &strm_flt(s)->filters, list) {
- if (IS_DATA_FILTER(f, chn))
- FLT_OFF(f, chn) = data;
- }
+ list_for_each_entry(f, &strm_flt(s)->filters, list)
+ FLT_OFF(f, chn) = data;
}
check_result:
@@ -894,12 +903,18 @@
ret = data = len - out;
DBG_TRACE_ENTER(STRM_EV_TCP_ANA|STRM_EV_FLT_ANA, s);
list_for_each_entry(filter, &strm_flt(s)->filters, list) {
- /* Call "data" filters only */
- if (!IS_DATA_FILTER(filter, chn))
+ unsigned long long *flt_off = &FLT_OFF(filter, chn);
+ unsigned int offset = *flt_off - *strm_off;
+
+ /* Call tcp_payload for filters only. Forward all data for
+ * others and update the filter offset
+ */
+ if (!IS_DATA_FILTER(filter, chn)) {
+ *flt_off += data - offset;
continue;
+ }
+
if (FLT_OPS(filter)->tcp_payload) {
- unsigned long long *flt_off = &FLT_OFF(filter, chn);
- unsigned int offset = *flt_off - *strm_off;
DBG_TRACE_DEVEL(FLT_ID(filter), STRM_EV_TCP_ANA|STRM_EV_FLT_ANA, s);
ret = FLT_OPS(filter)->tcp_payload(s, filter, chn, out + offset, data - offset);