On Wed, Oct 17, 2018 at 7:48 AM Ondrej Mosnacek <omosn...@redhat.com> wrote:
>
> We need to convert from little-endian before dong range checks on the
> ibpkey port numbers, otherwise we would be checking a wrong value.
>
> Fixes: 9fbb3112769a ("libsepol: Add ibpkey ocontext handling")
> Signed-off-by: Ondrej Mosnacek <omosn...@redhat.com>
> ---
>  libsepol/src/policydb.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c
> index a6d76ca3..dc201e2f 100644
> --- a/libsepol/src/policydb.c
> +++ b/libsepol/src/policydb.c
> @@ -2830,15 +2830,21 @@ static int ocontext_read_selinux(struct 
> policydb_compat_info *info,
>                                 break;
>                         case OCON_IBPKEY:
>                                 rc = next_entry(buf, fp, sizeof(uint32_t) * 
> 4);
> -                               if (rc < 0 || buf[2] > 0xffff || buf[3] > 
> 0xffff)
> +                               if (rc < 0)
>                                         return -1;
>
> +                               c->u.ibpkey.low_pkey  = le32_to_cpu(buf[2]);
> +                               c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);

Ahh you're assigning a 32 bit value to a 16 bit variable low|high_pkey. I think
you need to assign them to a uint32_t if you want to actually range check them.

> +
> +                               if (c->u.ibpkey.low_pkey  > 0xffff ||
> +                                   c->u.ibpkey.high_pkey > 0xffff)
> +                                       return -1;
> +
> +                               /* we want c->u.ibpkey.subnet_prefix in 
> network
> +                                * (big-endian) order, just memcpy it */
>                                 memcpy(&c->u.ibpkey.subnet_prefix, buf,
>                                        sizeof(c->u.ibpkey.subnet_prefix));
>
> -                               c->u.ibpkey.low_pkey = le32_to_cpu(buf[2]);
> -                               c->u.ibpkey.high_pkey = le32_to_cpu(buf[3]);
> -
>                                 if (context_read_and_validate
>                                     (&c->context[0], p, fp))
>                                         return -1;
> --
> 2.17.2
>
See job: https://travis-ci.org/SELinuxProject/selinux/jobs/442750208

Build fail with gcc:

policydb.c:2839:31: error: comparison is always false due to limited
range of data type [-Werror=type-limits]
     if (c->u.ibpkey.low_pkey  > 0xffff ||
                               ^
policydb.c:2840:31: error: comparison is always false due to limited
range of data type [-Werror=type-limits]
         c->u.ibpkey.high_pkey > 0xffff)
_______________________________________________
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.

Reply via email to