http-parser icon indicating copy to clipboard operation
http-parser copied to clipboard

Ensure host offset & length are initialized - fuzzer found possible leaks

Open zzo opened this issue 5 years ago • 5 comments

zzo avatar Jul 23 '18 17:07 zzo

this should be taken care of already by

http_parser_url_init(struct http_parser_url *u) {
  memset(u, 0, sizeof(*u));
}

ploxiln avatar Jul 23 '18 17:07 ploxiln

cool so http_parser_parse_url should call that?

zzo avatar Jul 23 '18 18:07 zzo

The user of the library should call that before calling http_parser_parse_url(). Or, alternatively, the struct http_parser_url might be a member of a larger struct that was memset() all at once, or was allocated with calloc(), in which case the redundant clearing could be skipped ...

If there's any "bug" here, it's that http_parser_parse_url() is only very briefly mentioned and is not really documented ... just the existence of http_parser_url_init(struct http_parser_url *u) is there to imply you should initialize the struct.

ploxiln avatar Jul 23 '18 18:07 ploxiln

ok cool of the 3 references to it being called only 1 in test.c doesn't do this: https://github.com/nodejs/node/blob/bc690e9ef52bebc34cad7ddb40e74472bcb272ca/deps/http_parser/test.c#L2614 not a big deal! still it couldn't hurt to 0 out the struct in case someone forgets to initialize it no? Otherwise just close & ignore this - thanks! Or I can create another PR that just calls http_parser_url_init from http_parser_parse_url - is there ever a case where you don't want to do this (except the case that maybe the caller has already done so)?

zzo avatar Jul 23 '18 18:07 zzo

Looks like a bug in the test to me. I'm not a maintainer though, just trying to help clarify the situation.

ploxiln avatar Jul 23 '18 18:07 ploxiln