Improve length checks in DHCP Options parsing of dhcpcd.
Bug: 26461634
Change-Id: I258b74c3f21bd312f0b7d2185b9cf419b3593637
diff --git a/dhcp.c b/dhcp.c
index bdd984f..b3f7cb1 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -271,21 +271,34 @@
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 */
