| Message ID | 20170921165958.3218-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix memory leaks and overreads in ofp-util | expand |
> On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote: > > A buffer overread of up to 4 bytes was possible given a malformed > message. The message was discarded following the overread. > > Found by libFuzzer. > > Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > lib/ofp-util.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > index 86dd5cb61653..e915cb2ab2d7 100644 > --- a/lib/ofp-util.c > +++ b/lib/ofp-util.c > @@ -10517,6 +10517,9 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, > msg->bundle_id = ntohl(m->bundle_id); > msg->flags = ntohs(m->flags); > > + if (b.size < sizeof(struct ofp_header)) { > + return OFPERR_OFPBFC_MSG_BAD_LEN; > + } Do you mind adding a brief comment indicating that this is checking the inner OpenFlow header? It will help people like me who miss that. :-) Acked-by: Justin Pettit <jpettit@ovn.org> Thanks, --Justin
On Fri, Sep 22, 2017 at 02:06:01PM -0700, Justin Pettit wrote: > > > On Sep 21, 2017, at 9:59 AM, Ben Pfaff <blp@ovn.org> wrote: > > > > A buffer overread of up to 4 bytes was possible given a malformed > > message. The message was discarded following the overread. > > > > Found by libFuzzer. > > > > Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > lib/ofp-util.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/lib/ofp-util.c b/lib/ofp-util.c > > index 86dd5cb61653..e915cb2ab2d7 100644 > > --- a/lib/ofp-util.c > > +++ b/lib/ofp-util.c > > @@ -10517,6 +10517,9 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, > > msg->bundle_id = ntohl(m->bundle_id); > > msg->flags = ntohs(m->flags); > > > > + if (b.size < sizeof(struct ofp_header)) { > > + return OFPERR_OFPBFC_MSG_BAD_LEN; > > + } > > Do you mind adding a brief comment indicating that this is checking the inner OpenFlow header? It will help people like me who miss that. :-) > > Acked-by: Justin Pettit <jpettit@ovn.org> Sure, I added some comments and applied this to master and then backported as far as 2.6.
diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 86dd5cb61653..e915cb2ab2d7 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -10517,6 +10517,9 @@ ofputil_decode_bundle_add(const struct ofp_header *oh, msg->bundle_id = ntohl(m->bundle_id); msg->flags = ntohs(m->flags); + if (b.size < sizeof(struct ofp_header)) { + return OFPERR_OFPBFC_MSG_BAD_LEN; + } msg->msg = b.data; if (msg->msg->version != oh->version) { return OFPERR_OFPBFC_BAD_VERSION;
A buffer overread of up to 4 bytes was possible given a malformed message. The message was discarded following the overread. Found by libFuzzer. Reported-by: Bhargava Shastry <bshastry@sec.t-labs.tu-berlin.de> Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/ofp-util.c | 3 +++ 1 file changed, 3 insertions(+)