nx_to_ofp_flow_update_event() aborts the execution if incorrect
event is passed, so checking has to be done before conversion
in order to avoid the crash while decoding malformed flow update
message:
==397030==ERROR: AddressSanitizer: ABRT on unknown address 0x... )
0 0x7fd26688418b in raise
1 0x7fd266863858 in abort
2 0x6a6cbd in nx_to_ofp_flow_update_event lib/ofp-monitor.c:399:9
3 0x6a6cbd in ofputil_decode_flow_update lib/ofp-monitor.c:856:25
4 0x56491d in ofp_print_flow_monitor_reply lib/ofp-print.c:779:22
5 0x55f0a0 in ofp_to_string__ lib/ofp-print.c:1154:16
6 0x55f0a0 in ofp_to_string lib/ofp-print.c:1244:21
7 0x5603a5 in ofp_print lib/ofp-print.c:1288:28
Credit to OSS-Fuzz.
Additionally removed the extra 'reply' word from the error message,
since ofpraw_get_name(raw) already has one.
Fixes: c3e64047d1cc ("ofp-monitor: Support flow monitoring for OpenFlow 1.3,
1.4+.")
Reported-at: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=47112
Signed-off-by: Ilya Maximets <[email protected]>
---
v2:
- Fixed the osx build failure (nx_event type mismatch in VLOG_WARN).
lib/ofp-monitor.c | 20 ++++++++++--------
tests/automake.mk | 1 +
tests/fuzz-regression-list.at | 1 +
.../ofp_print_fuzzer-4671928750702592 | Bin 0 -> 32 bytes
4 files changed, 13 insertions(+), 9 deletions(-)
create mode 100644 tests/fuzz-regression/ofp_print_fuzzer-4671928750702592
diff --git a/lib/ofp-monitor.c b/lib/ofp-monitor.c
index 1756c9e8e..c27733a52 100644
--- a/lib/ofp-monitor.c
+++ b/lib/ofp-monitor.c
@@ -847,19 +847,20 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
case OFPRAW_ONFST13_FLOW_MONITOR_REPLY:
case OFPRAW_NXST_FLOW_MONITOR_REPLY: {
struct nx_flow_update_header *nfuh;
+ enum nx_flow_update_event nx_event;
if (msg->size < sizeof(struct nx_flow_update_header)) {
goto bad_len;
}
nfuh = msg->data;
- update->event = nx_to_ofp_flow_update_event(ntohs(nfuh->event));
length = ntohs(nfuh->length);
if (length > msg->size || length % 8) {
goto bad_len;
}
- if (update->event == OFPFME_ABBREV) {
+ nx_event = ntohs(nfuh->event);
+ if (nx_event == NXFME_ABBREV) {
struct nx_flow_update_abbrev *nfua;
if (length != sizeof *nfua) {
@@ -868,10 +869,9 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
nfua = ofpbuf_pull(msg, sizeof *nfua);
update->xid = nfua->xid;
- return 0;
- } else if (update->event == OFPFME_ADDED
- || update->event == OFPFME_REMOVED
- || update->event == OFPFME_MODIFIED) {
+ } else if (nx_event == NXFME_ADDED
+ || nx_event == NXFME_DELETED
+ || nx_event == NXFME_MODIFIED) {
struct nx_flow_update_full *nfuf;
unsigned int actions_len;
unsigned int match_len;
@@ -926,12 +926,14 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
update->ofpacts = ofpacts->data;
update->ofpacts_len = ofpacts->size;
- return 0;
} else {
VLOG_WARN_RL(&rl, "NXST_FLOW_MONITOR reply has bad event %"PRIu16,
ntohs(nfuh->event));
return OFPERR_NXBRC_FM_BAD_EVENT;
}
+
+ update->event = nx_to_ofp_flow_update_event(nx_event);
+ return 0;
}
case OFPRAW_OFPST14_FLOW_MONITOR_REPLY: {
struct ofp_flow_update_header *ofuh;
@@ -1017,8 +1019,8 @@ ofputil_decode_flow_update(struct ofputil_flow_update
*update,
OVS_NOT_REACHED();
}
bad_len:
- VLOG_WARN_RL(&rl, "%s reply has %"PRIu32" leftover bytes at end",
- ofpraw_get_name(raw), msg->size);
+ VLOG_WARN_RL(&rl, "%s has %"PRIu32" leftover bytes at end",
+ ofpraw_get_name(raw), msg->size);
return OFPERR_OFPBRC_BAD_LEN;
}
diff --git a/tests/automake.mk b/tests/automake.mk
index 8a9151f81..34ddda6aa 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -117,6 +117,7 @@ FUZZ_REGRESSION_TESTS = \
tests/fuzz-regression/flow_extract_fuzzer-5457710546944000 \
tests/fuzz-regression/json_parser_fuzzer-4790908707930112 \
tests/fuzz-regression/ofp_print_fuzzer-4584019764183040 \
+ tests/fuzz-regression/ofp_print_fuzzer-4671928750702592 \
tests/fuzz-regression/ofp_print_fuzzer-4730143510626304 \
tests/fuzz-regression/ofp_print_fuzzer-4854119633256448 \
tests/fuzz-regression/ofp_print_fuzzer-5070973479944192 \
diff --git a/tests/fuzz-regression-list.at b/tests/fuzz-regression-list.at
index 2347c690e..247bb667a 100644
--- a/tests/fuzz-regression-list.at
+++ b/tests/fuzz-regression-list.at
@@ -2,6 +2,7 @@ TEST_FUZZ_REGRESSION([flow_extract_fuzzer-5112775280951296])
TEST_FUZZ_REGRESSION([flow_extract_fuzzer-5457710546944000])
TEST_FUZZ_REGRESSION([json_parser_fuzzer-4790908707930112])
TEST_FUZZ_REGRESSION([ofp_print_fuzzer-4584019764183040])
+TEST_FUZZ_REGRESSION([ofp_print_fuzzer-4671928750702592])
TEST_FUZZ_REGRESSION([ofp_print_fuzzer-4730143510626304])
TEST_FUZZ_REGRESSION([ofp_print_fuzzer-4854119633256448])
TEST_FUZZ_REGRESSION([ofp_print_fuzzer-5070973479944192])
diff --git a/tests/fuzz-regression/ofp_print_fuzzer-4671928750702592
b/tests/fuzz-regression/ofp_print_fuzzer-4671928750702592
new file mode 100644
index
0000000000000000000000000000000000000000..5d53b1247c89bfa1a5d5c3e0b9cfec75f06e5866
GIT binary patch
literal 32
ccmZP+WKaNt{~*A?psc{az`*n$!u=1T0m#e{2><{9
literal 0
HcmV?d00001
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev