From 3c47c99731e7adff63e05ffbe7b8840abc5708e5 Mon Sep 17 00:00:00 2001 From: Pierre Imai Date: Sun, 14 Feb 2016 23:19:22 -0800 Subject: Improve length checks in DHCP Options parsing of dhcpcd. Bug: 26461634 Change-Id: I258b74c3f21bd312f0b7d2185b9cf419b3593637 --- dhcp.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/dhcp.c b/dhcp.c index bdd984f..b3f7cb1 100644 --- a/dhcp.c +++ b/dhcp.c @@ -271,21 +271,34 @@ valid_length(uint8_t option, int dl, int *type) if (type) *type = opt->type; - + /* The size of RFC3442 and RFC5969 options is checked at a later + * stage in the code */ if (opt->type == 0 || opt->type & (STRING | RFC3442 | RFC5969)) return 0; - + /* The code does not use SINT16 / SINT32 together with ARRAY. + * It is however far easier to reason about the code if all + * possible array elements are included, and also does not code + * any additional CPU resources. sizeof(uintXX_t) == + * sizeof(intXX_t) can be assumed. */ sz = 0; - if (opt->type & (UINT32 | IPV4)) + if (opt->type & (UINT32 | SINT32 | IPV4)) sz = sizeof(uint32_t); - if (opt->type & UINT16) + else if (opt->type & (UINT16 | SINT16)) sz = sizeof(uint16_t); - if (opt->type & UINT8) + else if (opt->type & UINT8) sz = sizeof(uint8_t); - if (opt->type & (IPV4 | ARRAY)) - return dl % sz; - return (dl == sz ? 0 : -1); + if (opt->type & ARRAY) { + /* The result of modulo zero is undefined. There are no + * options defined in this file that do not match one of + * the if-clauses above, so the following is not really + * necessary. However, to avoid confusion and unexpected + * behavior if the defined options are ever extended, + * returning false here seems sensible. */ + if (!sz) return -1; + return (dl % sz == 0) ? 0 : -1; + } + return (sz == dl) ? 0 : -1; } /* unknown option, so let it pass */ -- cgit v1.2.3