On Sun, Nov 30, 2014 at 11:42:19AM -0800, Kevin J. McCarthy wrote: > The first change, while fixing the logic error causing the CVE, may > cause problems with all the other calls to skip_email_wsp() that relied > on a broad definition of whitespace. I don't know - there are too many > calls and I'm not familiar enough with all the code to know.
I responded to this on the ticket, but after reviewing the code, it appears that all header lines are unfolded at the point that skip_email_ws() is being used. It used to be that we didn't always unfold, so the SKIPWS() macro considered newline to be a whitespace so it didn't prematurely terminate processing folded lines. > The second change may or may not be palatable to the core devs. It > stops a segfault, but hides a logic error. I still need to review this. > It looks like the quickest change would be to revert sendlib.c back to > using: > while (*t == ' ' || *t == '\t') > t++; This is probably the safest option, at least until we are certain that changing the semantics of skip_email_ws() is not harmful. It will solve the crash, and not introduce any other new behavior. I'd suggest pushing this change to stable, and the skip_email_ws() change to the dev branch. > Long term, it would be better create two functions: a strict WSP and a > loose whitespace one, and tease apart what needs which. Based on my review, I don't think separate functions are necessary. me
