On Tue, 27 Sep 2016 11:19:21 +0200 Michał Rzepka <mrze...@student.agh.edu.pl> wrote:
> Recently, I discovered major multipart message parser flaw. The issue > was observed while testing Aggregate Flow Statistics message in OpenFlow > 1.5 and Open vSwitch. Similar (and potentially also vulnerable) code > snippets are also present in other message parsers (e.g. OFPHello). I'd > like to ask for opinions on proposed solution. If accepted, similar > patches should also be applied for other message parsers. > > Brief description (steps to reproduce the issue): > 1. REST API is called to retrieve aggregate flow stats: > curl http://localhost:8080/stats/aggregateflow/8796750139643 > 2. Open vSwitch replies to Aggregate Stats Request with Aggregate Stats > Reply: > message buffer: 0x06 0x13 0x00 0x28 0x53 0xfe 0xc4 0xaf 0x00 0x02 0x00 > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 > 0x00 > (note that due to incomplete OF 1.5 support in OvS, message is > malformed - ofp_stats struct filled with zeros) > 3. Message is processed by Ryu parsers: > ofproto_parser.msg -> ofproto_v1_5_parser.msg_parser -> > ofproto_v1_5_parser.OFPMultipartReply.parser > 4. Here, message body contents are parsed > (ofproto_v1_5_parser.OFPMultipartReply.parser, lines 1858-1861): > while offset < msg_len: > b = stats_type_cls.cls_stats_body_cls.parser(msg.buf, offset) > body.append(b) > offset += b.length if hasattr(b, 'length') else b.len > 5. Due to incorrect message format, zero-filled message part is parsed > as b=OFPAggregateStats(length=0,stats=OFPStats(oxs_fields={})), > resulting in constant offset value, as in each iteration offset += 0. > 6. Parser remains trapped in a infinite loop with offset = 16, msg_len = > 40. Ryu controller hangs completely. > > OFPMultipartReply parser was observed to handle malformed messages > improperly. The patch introduces offset check to fix processing of > malformed messages in ofproto_v1_5_parser.OFPMultipartReply.parser. > > Signed-off-by: Michal Rzepka <mrze...@student.agh.edu.pl> > --- > ryu/ofproto/ofproto_v1_5_parser.py | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) Applied, thanks. Surely, similar patches are welcome. ------------------------------------------------------------------------------ _______________________________________________ Ryu-devel mailing list Ryu-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ryu-devel
