If enough large input is given ofpact_finish will fail.
Implemented ofpbuf_oversized function to check for oversized
buffer. Checks were added for parse functions and error messages
returned.

Basic manual testing performed.

Reported-by:
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12972
Signed-off-by: Toms Atteka <cpp.code...@gmail.com>

v1->v2: added over sized check as a separate function and added
checks for other parse functions where they might fail.
---
 include/openvswitch/ofpbuf.h |  6 ++++++
 lib/bundle.c                 |  5 +++++
 lib/learn.c                  |  5 +++++
 lib/ofp-actions.c            | 29 +++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+)

diff --git a/include/openvswitch/ofpbuf.h b/include/openvswitch/ofpbuf.h
index e4cf088..1136ba0 100644
--- a/include/openvswitch/ofpbuf.h
+++ b/include/openvswitch/ofpbuf.h
@@ -162,6 +162,7 @@ char *ofpbuf_to_string(const struct ofpbuf *, size_t 
maxbytes);
 static inline struct ofpbuf *ofpbuf_from_list(const struct ovs_list *);
 void ofpbuf_list_delete(struct ovs_list *);
 static inline bool ofpbuf_equal(const struct ofpbuf *, const struct ofpbuf *);
+static inline bool ofpbuf_oversized(const struct ofpbuf *ofpacts);
 
 
 /* Frees memory that 'b' points to, as well as 'b' itself. */
@@ -272,6 +273,11 @@ static inline bool ofpbuf_equal(const struct ofpbuf *a, 
const struct ofpbuf *b)
            memcmp(a->data, b->data, a->size) == 0;
 }
 
+static inline bool ofpbuf_oversized(const struct ofpbuf *ofpacts)
+{
+    return (char *)ofpbuf_tail(ofpacts) - (char *)ofpacts->header > UINT16_MAX;
+}
+
 #ifdef  __cplusplus
 }
 #endif
diff --git a/lib/bundle.c b/lib/bundle.c
index 558e890..edb11f6 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -183,6 +183,11 @@ bundle_parse__(const char *s, const struct 
ofputil_port_map *port_map,
         bundle = ofpacts->header;
         bundle->n_slaves++;
     }
+
+    if (ofpbuf_oversized(ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_BUNDLE(ofpacts, &bundle);
     bundle->basis = atoi(basis);
 
diff --git a/lib/learn.c b/lib/learn.c
index 642ce18..a40209e 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -455,6 +455,11 @@ learn_parse__(char *orig, char *arg, const struct 
ofputil_port_map *port_map,
             learn = ofpacts->header;
         }
     }
+
+    if (ofpbuf_oversized(ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_LEARN(ofpacts, &learn);
 
     return NULL;
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index e8e88ac..1340614 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -989,6 +989,11 @@ parse_CONTROLLER(char *arg, const struct 
ofpact_parse_params *pp)
             controller = pp->ofpacts->header;
             controller->userdata_len = userdata_len;
         }
+
+        if (ofpbuf_oversized(pp->ofpacts)) {
+            return xasprintf("input too big");
+        }
+
         ofpact_finish_CONTROLLER(pp->ofpacts, &controller);
     }
 
@@ -3690,6 +3695,11 @@ parse_DEC_TTL(char *arg, const struct 
ofpact_parse_params *pp)
             return xstrdup("dec_ttl_cnt_ids: expected at least one controller "
                            "id.");
         }
+
+        if (ofpbuf_oversized(pp->ofpacts)) {
+            return xasprintf("input too big");
+        }
+
         ofpact_finish_DEC_TTL(pp->ofpacts, &ids);
     }
     return NULL;
@@ -4443,6 +4453,11 @@ parse_ENCAP(char *arg, const struct ofpact_parse_params 
*pp)
     /* ofpbuf may have been re-allocated. */
     encap = pp->ofpacts->header;
     encap->n_props = n_props;
+
+    if (ofpbuf_oversized(pp->ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_ENCAP(pp->ofpacts, &encap);
     return NULL;
 }
@@ -5772,6 +5787,11 @@ parse_NOTE(const char *arg, const struct 
ofpact_parse_params *pp)
     struct ofpact_note *note = ofpbuf_at_assert(pp->ofpacts, start_ofs,
                                                 sizeof *note);
     note->length = pp->ofpacts->size - (start_ofs + sizeof *note);
+
+    if (ofpbuf_oversized(pp->ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_NOTE(pp->ofpacts, &note);
     return NULL;
 }
@@ -5929,6 +5949,10 @@ parse_CLONE(char *arg, const struct ofpact_parse_params 
*pp)
     pp->ofpacts->header = ofpbuf_push_uninit(pp->ofpacts, sizeof *clone);
     clone = pp->ofpacts->header;
 
+    if (ofpbuf_oversized(pp->ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_CLONE(pp->ofpacts, &clone);
     ofpbuf_push_uninit(pp->ofpacts, clone_offset);
     return error;
@@ -6615,6 +6639,11 @@ parse_CT(char *arg, const struct ofpact_parse_params *pp)
     if (!error && oc->flags & NX_CT_F_FORCE && !(oc->flags & NX_CT_F_COMMIT)) {
         error = xasprintf("\"force\" flag requires \"commit\" flag.");
     }
+
+    if (ofpbuf_oversized(pp->ofpacts)) {
+        return xasprintf("input too big");
+    }
+
     ofpact_finish_CT(pp->ofpacts, &oc);
     ofpbuf_push_uninit(pp->ofpacts, ct_offset);
     return error;
-- 
2.7.4

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to