diff options
author | Pierre Imai <imaipi@google.com> | 2016-02-14 23:19:22 -0800 |
---|---|---|
committer | The Android Automerger <android-build@google.com> | 2016-02-26 16:55:49 -0800 |
commit | 3c47c99731e7adff63e05ffbe7b8840abc5708e5 (patch) | |
tree | 95e001b9a2a3e4ef6b20254a8e8cda30fe96ef0c | |
parent | 782285b3be6fd647284db735010ae0cbd55529bd (diff) | |
download | dhcpcd-marshmallow-mr3-release.tar.gz |
Improve length checks in DHCP Options parsing of dhcpcd.android-6.0.1_r79android-6.0.1_r78android-6.0.1_r77android-6.0.1_r74android-6.0.1_r72android-6.0.1_r70android-6.0.1_r66android-6.0.1_r61android-6.0.1_r60android-6.0.1_r59android-6.0.1_r58android-6.0.1_r52android-6.0.1_r51android-6.0.1_r50android-6.0.1_r49android-6.0.1_r46android-6.0.1_r43android-6.0.1_r42android-6.0.1_r41android-6.0.1_r40android-6.0.1_r30marshmallow-mr3-release
Bug: 26461634
Change-Id: I258b74c3f21bd312f0b7d2185b9cf419b3593637
-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 */ |