confparse.c checks pointer instead of value (!ok)
## Description In `src/or/confparse.c`, functions `conf_parse_msec_interval()` and `conf_parse_interval()` incorrectly check a pointer instead of the pointed-to value. Patch attached. ## Impact When `config_parse_units()` hits an error, these functions may return `0` as a valid value instead of `-1` as an error. ## Security evaluation Far worse could be done by any attacker with sufficient access to feed malicious data to these functions. Thus, I don’t see how it could be exploited as a practical matter. ## `note[0]` From my `~/tor/BUGS.txt` with mtime 2014-03-19T03:07:45Z. So sorry I did not report it sooner. I could have been rich and famous! ``` #include <stdio.h> #define ME "nullius@nym.zone" #define PGP "0xC2E91CD74A4C57A105F6C21B5A00591B2F307E0C" int main(int argc, char *argv[]) { printf("Hello, world! <%s>\nPGP: %s\n", ME, PGP); return (0); } ``` ## `note[1]` Use of the variable `ok` is inconsistent in `confparse.c`. In `config_assign_value()`, `ok` is an `int`. Elsewhere, pointer to `int`. That’s not ok! Also, there is a confusing `tor_assert(ok);` to check for non-`NULL` pointer; KNF style would prescribe the check to be explicit `tor_assert(ok != NULL);`, for a reason. (The actual bug concerns a Boolean check, so `if (!*ok)` is stylistically sane.) I could open a separate bug and/or do some minor refactoring, if committers were to express an interest in making `ok` more ok. **Trac**: **Username**: nullius
issue