> I found four problems and it’s clear the out of memory issue is not a bug.
Thanks for the reports, the diffs and the concise summaries. I think that building a tool that is capable of finding such bugs is pretty cool and if you have a short writeup on what it does I'd be interested. If you find more bugs please do send them to us. > The other three reports are, stripped of all the gobbley gook: > > 1. ccr.c:1283 — parse_aspa_providers() checks p_num <= 0 but not p_num >= > MAX_ASPA_PROVIDERS. The equivalent function in aspa.c (line 66) has the > upper-bound check so I flagged this as an inconsistency during code review. The fork bomb and CCR ones have already been discussed. They both only concern file mode. > 2. http.c:1631 — http_redirect() is called when http_isredirect() returns > true (status code only), but conn->redir_uri is only set when a Location: > header is parsed. A 301 with no Location header passes NULL through > http_info() to strnvis(). Fix: check conn->redir_uri != NULL before calling > http_redirect(). Definitely a valid finding. The suggested fix is one way but not ideal since it's done a bit late. We decided to stop processing the request a bit earlier: https://github.com/openbsd/src/commit/f97bb3898e4d5eef036c382a0c37dcd75c914318 > 3. as.c:75-7: as_check_overlap() uses `ases->range.min` and > `ases->range.max` instead of `as->range.min` and `as->range.max`. Compares > ases[0] against ases[i] instead of the incoming entry against ases[i]. Every > other branch in the function uses `as->`. > I see no reason why we’d check ases->range.min instead of as->range.min as > everywhere else (which is why I see this as a bug). Absolutely. Dumb bug and your fix was committed as you sent it: https://github.com/openbsd/src/commit/566debf87d661c2ed816de66da56b8d23eb76465 We will cut a new release with rpki-client shortly anyway and I don't think the findings are reason enough to expedite a release. > Feel free to be brutal with your feedback. Honestly I’m going to get a kick > out of it (used to read some of your comments on the mailing list in college > before I stopped using OpenBSD). > > These reports were found with a tool I’ve built for linting that goes through > entire codebases looking for inconsistencies that are potential issues, which > I get in a report. Happy to tell you about it if you’re interested, but > these were reviewed by me, for what that’s worth. Which maybe is not much at > midnight... You were absolutely right to report these and including analysis and suggested fixes is fantastic. I for one appreciate it. Usually short summaries like in your last mail are enough for us to figure out what's needed, especially if you already have a suggested fix in diff form. If not, we'll ask. One reason why people reacted so antagonistically is due to the extremely long-winded form. We get a number of such generated reports every day on the private security lists. It's frankly frustrating and quite exhausting to plow through the robots' logorrhea confidently asserting half-truths and sheer nonsense in order to figure out if there's anything of value hiding in there. More often than not there actually isn't. I understand that Pascal's bonmot on not having the time to be shorter applies in this day and age where text that looks convincing at a glance is a prompt or two away. However, many people report things to bolster their CVs, so the fact that they take such a shortcut and thereby significantly increase the already unpleasant work of the people at the receiving end just isn't ideal. Presumably the correct way to handle such reports is to feed them right back to the machines to get a summary, but that such a step should even be considered indicates that something's wrong.
