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