From: Ben Pfaff <b...@ovn.org>
A crafted MPLS packet yields a zero 'count' in this excerpt from
miniflow_extract():
count = parse_mpls(&data, &size);
miniflow_push_words_32(mf, mpls_lse, mpls, count);
In turn, miniflow_push_words_32() updated mf.map as follows:
MF.map |= ((UINT64_MAX >> (64 - DIV_ROUND_UP(N_WORDS, 2))) << ofs64);
which expanded to:
mf.map |= (UINT64_MAX >> 64) << ofs64;
Unforunately, C renders shifting a 64-bit constant by 64 bits undefined.
On common x86 platforms, 'n << 64' is equal to 'n', so this behaves as:
mf.map |= UINT64_MAX << ofs64;
In this particular case, ofs64 is 15, so this sets the most-significant 48
bits of mf.map (a 63-bit bit-field) to 1. Only the least-significant 28
bits of mf.map should ever be set to 1, so this sets 35 bits to 1 that
should never be. Because of the structure of the data structure that
mf.map is embedded within, this makes it possible later to overwrite 8*35
== 280 bytes of data in the stack. However, there is no obvious way to
control the data used in the overwrite--it is memcpy'd from one place to
another but the source data does not come from the network. In the bug
reporter's testing, this overwrite caused a userspace crash if debug
logging was enabled, but not otherwise.
This commit fixes the problem by avoiding the out-of-range shift.
Vulnerability: CVE-2016-2074
Reported-by: Kashyap Thimmaraju <kashyap.thimmar...@sec.t-labs.tu-berlin.de>
Reported-by: Bhargava Shastry <bshas...@sec.t-labs.tu-berlin.de>
Signed-off-by: Ben Pfaff <b...@ovn.org>
Acked-by: Jesse Gross <je...@kernel.org>
---
lib/flow.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 5df23a9..03c175a 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira,
Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -197,7 +197,7 @@ BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will
have runtime "
/* Data at 'valuep' may be unaligned. */
#define miniflow_push_words_(MF, OFS, VALUEP, N_WORDS) \
-{ \
+if (N_WORDS) { \
int ofs64 = (OFS) / 8; \
\
MINIFLOW_ASSERT(MF.data + (N_WORDS) <= MF.end && (OFS) % 8 == 0 \
@@ -210,7 +210,7 @@ BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will
have runtime "
/* Push 32-bit words padded to 64-bits. */
#define miniflow_push_words_32_(MF, OFS, VALUEP, N_WORDS) \
-{ \
+if (N_WORDS) { \
int ofs64 = (OFS) / 8; \
\
MINIFLOW_ASSERT(MF.data + DIV_ROUND_UP(N_WORDS, 2) <= MF.end \
--
1.9.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
