Fix undefined behaviour in X509_NAME_cmp() #6291
Conversation
If the lengths of both names is 0 then don't attempt to do a memcmp. Issue reported by Simon Friedberger, Robert Merget and Juraj Somorovsky.
mattcaswell
added
branch: master
|
Appveyor red cross appears unrelated. |
| @@ -173,7 +173,7 @@ int X509_NAME_cmp(const X509_NAME *a, const X509_NAME *b) | |||
|
|
|||
| ret = a->canon_enclen - b->canon_enclen; | |||
|
|
|||
| if (ret) | |||
| if (ret != 0 || a->canon_enclen == 0) | |||
There was a problem hiding this comment.
While we are at it, wouldn't it be appropriate to harmonize even ![ab]->cannon_enc checks? Well, one can say that since it goes all the way down to 1.0.2, then it might be not as appropriate... Side note then?
On another side note, function shamelessly breaks const contract, so it would be only appropriate to omit const qualifier [in next release]. But if considered inappropriate, then one can wonder if this contradiction should be resolved already now.
BTW, there is related "philosophical" question. While modification preserves original behaviour, one can wonder if it would appropriate to consider empty strings to be non-equal. Unless a and b are same. I mean empty string in one object can be viewed as non-equal to another [empty string]. Rationale is that empty string would indicate an impossible value, and comparison of impossible values is meaningless. For example 1/x and 1/x^2 for x=0 are both impossible, but are they equal? The question is rhetorical.
There was a problem hiding this comment.
Side note then?
I'll take it as a side note :-)
There should have been alert that was not observed. It's not big mystery on how to solve it. In sense that it's bigger mystery why failure is so seldom. It's second time I note it since big TLSProxy overhaul. Well, I might have missed another or couple... |
richsalz
added
the
approval: done
|
Pushed. Thanks. |
If the lengths of both names is 0 then don't attempt to do a memcmp. Issue reported by Simon Friedberger, Robert Merget and Juraj Somorovsky. Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> (Merged from #6291)
If the lengths of both names is 0 then don't attempt to do a memcmp.
Issue reported by Simon Friedberger, Robert Merget and Juraj Somorovsky.