[3/5] libsepol: ebitmap: reject loading bitmaps with incorrect high bit

Commit Message

Nicolas Iooss Nov. 23, 2016, 10:06 p.m. UTC

Currently ebitmap_load() accepts loading a bitmap with highbit=192 and
one node {startbit=0, map=0x2}. When iterating over the bitmap,
ebitmap_for_each_bit() is expected to only yield "1" but it gives the
following bits: 1, 65, 129.

This is due to two facts in ebitmap_for_each_bit() implementation:
* ebitmap_next() stays on the first (and only) node of the bitmap
  instead of stopping the iteration.
* the end condition of the for loop consists in comparing the bit with
  ebitmap_length() (ie. the bitmap highbit), which is above the limit of
  the last node here.

These are not bugs when the bitmap highbit is equals to
l->startbit+MAPSIZE, where l is the last node (this is how
ebitmap_set_bit() sets it). So a simple fix consists in making
ebitmap_load() reject bitmaps which are loaded with an invalid highbit
value.

This issue has been found while fuzzing semodule_package with the
American Fuzzy Lop.

Signed-off-by: Nicolas Iooss <[e-mail]>
---
 libsepol/src/ebitmap.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Stephen Smalley Nov. 28, 2016, 2:21 p.m. UTC | #1

On 11/23/2016 05:06 PM, Nicolas Iooss wrote:
> Currently ebitmap_load() accepts loading a bitmap with highbit=192 and
> one node {startbit=0, map=0x2}. When iterating over the bitmap,
> ebitmap_for_each_bit() is expected to only yield "1" but it gives the
> following bits: 1, 65, 129.
> 
> This is due to two facts in ebitmap_for_each_bit() implementation:
> * ebitmap_next() stays on the first (and only) node of the bitmap
>   instead of stopping the iteration.
> * the end condition of the for loop consists in comparing the bit with
>   ebitmap_length() (ie. the bitmap highbit), which is above the limit of
>   the last node here.
> 
> These are not bugs when the bitmap highbit is equals to
> l->startbit+MAPSIZE, where l is the last node (this is how
> ebitmap_set_bit() sets it). So a simple fix consists in making
> ebitmap_load() reject bitmaps which are loaded with an invalid highbit
> value.
> 
> This issue has been found while fuzzing semodule_package with the
> American Fuzzy Lop.

Thanks, applied

> 
> Signed-off-by: Nicolas Iooss <[e-mail]>
> ---
>  libsepol/src/ebitmap.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
> index fe8beb879a18..218adc293aae 100644
> --- a/libsepol/src/ebitmap.c
> +++ b/libsepol/src/ebitmap.c
> @@ -453,6 +453,12 @@ int ebitmap_read(ebitmap_t * e, void *fp)
>  
>  		l = n;
>  	}
> +	if (count && l->startbit + MAPSIZE != e->highbit) {
> +		printf
> +		    ("security: ebitmap: hight bit %u has not the expected value %zu\n",
> +		     e->highbit, l->startbit + MAPSIZE);
> +		goto bad;
> +	}
>  
>        ok:
>  	rc = 0;
>

Patch

diff --git a/libsepol/src/ebitmap.c b/libsepol/src/ebitmap.c
index fe8beb879a18..218adc293aae 100644
--- a/libsepol/src/ebitmap.c
+++ b/libsepol/src/ebitmap.c
@@ -453,6 +453,12 @@  int ebitmap_read(ebitmap_t * e, void *fp)
 
 		l = n;
 	}
+	if (count && l->startbit + MAPSIZE != e->highbit) {
+		printf
+		    ("security: ebitmap: hight bit %u has not the expected value %zu\n",
+		     e->highbit, l->startbit + MAPSIZE);
+		goto bad;
+	}
 
       ok:
 	rc = 0;