Phillip Wood <phillip.wood123@xxxxxxxxx> writes: > This probably doesn't matter in practice but we define our own > isspace() that does not treat '\v' and '\f' as whitespace. However > parse_timestamp() (which is just strtoumax()) uses the standard > library's isspace() which does treat those characters as whitespace > and is locale dependent. This means we can potentially stop at a > character that parse_timestamp() treats as whitespace and if there are > no digits after it we'll still walk past the end of the line. Using > Rene's suggestion of testing the character with isdigit() would fix > that. It would also avoid parsing negative timestamps as positive > numbers and reject any timestamps that begin with a locale dependent > digit. A very interesting observation. I wonder if a curious person can craft a malformed timestamp with "hash-object --literally" to do more than DoS themselves? We are not going to put anything other than [ 0-9+-] after the '>' we scan for, and making sure '>' is followed by SP and then [0-9] would be sufficient to ensure strtoumax() to stop before the '\n' but does not ensure that the "signal a bad timestamp with 0" happens. Perhaps that would be sufficient. I dunno. > I'm not familiar with this code, but would it be worth changing > parse_timestamp() to stop parsing if it sees a newline? Meaning replace or write our own strtoumax() equivalent?
