On 10/17/2018 05:18 PM, Paul Moore wrote:
On Wed, Oct 17, 2018 at 12:07 PM William Roberts
<bill.c.roberts@xxxxxxxxx> wrote:
On Wed, Oct 17, 2018 at 7:48 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> 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 <omosnace@xxxxxxxxxx>
---
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 you can, give me a chance to look over the kernel changes first. I
doubt I'll see anything objectionable given the review the patches
have already seen, but one never knows.
The kernel patch has the same bug, so it will also need a re-spin. Good
catch.
+
+ 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@xxxxxxxxxxxxx
To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
