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

Reply via email to