diff options
author | Pierre Imai <imaipi@google.com> | 2016-02-14 23:19:22 -0800 |
---|---|---|
committer | The Android Automerger <android-build@google.com> | 2016-03-01 13:17:15 -0800 |
commit | 94a7bd8ac4a4e6e3e7835e9fd7414b76af7aab8f (patch) | |
tree | 95e001b9a2a3e4ef6b20254a8e8cda30fe96ef0c | |
parent | 782285b3be6fd647284db735010ae0cbd55529bd (diff) | |
download | dhcpcd-marshmallow-dr-dragon-release.tar.gz |
Improve length checks in DHCP Options parsing of dhcpcd.android-6.0.1_r63android-6.0.1_r55android-6.0.1_r33android-6.0.1_r32android-6.0.1_r31marshmallow-dr-dragon-release
Bug: 26461634
Change-Id: I258b74c3f21bd312f0b7d2185b9cf419b3593637
(cherry picked from commit 6a26a2d69ed23acec290c83c104557f7e7e969d8)
-rw-r--r-- | dhcp.c | 29 |
1 files changed, 21 insertions, 8 deletions
@@ -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 */ |