patch for CVE-2012-2110 - incorrect?

[jsmith6134]

I was examining the patch for security fix for 5.0:

http://ftp.openbsd.org/pub/OpenBSD/p...ibcrypto.patch

part of the patch looks incorrect:

- if (!BUF_MEM_grow_clean(b,len+want))
+ if (len + want < len || !BUF_MEM_grow_clean(b,len+want))

"len + want < len" should always be false unless "want" can be negative. If "want" could be negative and that is what the author was trying to detect, then the code should be written:

+ if (want < 0 || !BUF_MEM_grow_clean(b,len+want))

I realize the patch does not show the full source for the file. Am I missing something?

[ocicat]

Originally Posted by jsmith6134

"len + want < len" should always be false unless "want" can be negative. If "want" could be negative and that is what the author was trying to detect, then the code should be written:

+ if (want < 0 || !BUF_MEM_grow_clean(b,len+want))

I realize the patch does not show the full source for the file. Am I missing something?

Fuller context can be gained by looking at the entire file:

http://www.openbsd.org/cgi-bin/cvswe...otate=1.5.16.1

Having spent only a few minutes looking through the file, it appears that there are overflow conditions the author wants to track as part of error handling. As for whether the expression len + want < len can be simplified, you should contact the author. Alternatively, you could install -current, modify the code, test, & then post the diff on tech@ for comments.

[jggimi]

I realize the patch does not show the full source for the file. Am I missing something?

1. In order to apply the patch, you'll need that source.
2. Few developers are active on this forum; and only a small subset of the developers work on crypto components.
3. In my opinion, you might get better results by discussing your concern either on the misc@ mailing list; or perhaps contacting the patch author directly -- Damien Miller (djm@).

----
Ocicat jumped in while I was typing. He may be right that tech@ is more appropriate, as yours is a code-specific question. But I wouldn't post there, myself, unless I was including a patch along with my post. Of these two mailing lists, misc@ seems more applicable for questions, tech@ for patches for testing and analysis.


Personally, I would probably just contact Damien directly if this were my question.