smartlist_bsearch_idx() is broken for short lists
Per asn: ---begin quote--- Hi Andrea, this is a possible bug I was discussing with Nick. He is pretty busy these days, so a third set of eyes could be useful: <asn> hi <asn> fwiw smartlist_bsearch_idx() seems a bit sloppy <asn> it doesn't handle the case where the sl is empty (smartlist_len(sl) - 1, underflows) <asn> and if sl has one element, there is still the danger of underflowing 'hi = mid-1;'. <asn> from what I see, the function is only used with smartlist carrying the whole routerlist, so it's "safe" till tor has only one relay. <nickm> ...at which point we've got other problems, yeah. <nickm> still a good idea to fix it <nickm> hang on <nickm> it's used in smartlist_bsearch, which is used in other places too <asn> i think smartlist_bsearch() is also only used with the whole routerlist. <nickm> you mean networkstatus <nickm> the routerlist is the list of routerinfo_t we know <nickm> there are enough places where it's used that I think we should have more eyes looking at it before we accidentally 0day ourselves. I'll look through the code by thursday; you can also ask athena on #tor-internal if you like <asn> btw, the interface of smartlist_bsearch_idx() doesn't allow particularly elegant error handling :( --- end quote --- This function is broken for lists of length zero or one and doesn't check the pointer arguments for nullness properly.
issue