Skip to content

Commit

Permalink
lib: added check to prevent int overflow
Browse files Browse the repository at this point in the history
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.

Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Reported-by: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12972
Signed-off-by: Toms Atteka <cpp.code.lv@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
TomCodeLV authored and blp committed Mar 26, 2019
1 parent 723b6ab commit 8e73833
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 0 deletions.
6 changes: 6 additions & 0 deletions include/openvswitch/ofpbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions lib/bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions lib/learn.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
29 changes: 29 additions & 0 deletions lib/ofp-actions.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 8e73833

Please sign in to comment.