This is indeed quite concerning. If we were to face a 5.7 or 6.0 release soon I'd say we should hold out, but in the end I'm also leaning more towards (2).
The reasons are two-fold: This is an issue, which may be exploitable in some (selected) situations and as a well-known crypto library we have a responsibility to patch as fast as possible and to ensure our library is secure. The second reason is that we want the 5.6.X branch to work *securely* until 5.7 or 6.0 kicks in. I think this by itself warrants a 5.6.4 release. The only reason I can imagine speaking against 5.6.4 is that Jeffrey would be put at too much load having to undergo the complete release testing and related activities for such a "minor" issue (and maybe the stuff we / he has fixed since the 5.6.3 release). BR JPM Am 28.12.2015 um 02:21 schrieb Jeffrey Walton: > Hi Everyone, > > *** Background *** > > A question was asked on Stack Overflow that caused us to look at > SecBlock's append operations. The SO question is at > http://stackoverflow.com/q/34479894. The SO question also triggered a > bug report on GitHub. The bug report is at > https://github.com/weidai11/cryptopp/issues/92. > > The append operations are 'operator +=' and 'operator +'. The old code > is at > http://www.cryptopp.com/docs/ref562/secblock_8h_source.html#l00331, > and the updated code is at > https://www.cryptopp.com/docs/ref563/secblock_8h_source.html#l00563. > They are effectively the same code modulo a size check and an assert. > > It appears 'operator +=' does not hand self-concatenation well, and it > triggers an assert in misc.h for debug build. That is, the following code: > > SecByteBlock t; > ... > t += t; > ... > > The code will break because when 'this' is grown, both 'this' and 't' > is grown. After t is grown, we attempt to copy too many bytes: > > memcpy_s > <https://www.cryptopp.com/docs/ref563/misc_8h.html#a5f6fcbaaf6f72fe46a6d911b033dfea0>(result.m_ptr+m_size, > (t.m_size-m_size)*sizeof(T), t.m_ptr, t.m_size*sizeof(T)); > > That is 't.m_size' has effectively doubled in size *after* we > performed the Grow(). > > *** memcpy_s *** > > The mitigating circumstance is 'memcpy_s'. You can see the source at > https://www.cryptopp.com/docs/ref563/misc_8h_source.html#l00278 . > > On non-Window machines, it will throw an InvalidArgument > <https://www.cryptopp.com/docs/ref563/class_invalid_argument.html>. > > Now, on Windows machines, Crypto++ memcpy_s or Windows memcpy_s may be > called, depending on the user's configuration. That means the > exception may not be thrown in some configurations. If the exception > is *not* thrown, then I believe it results in a silent truncation. > > We don't know if anyone is actually using the code and possibly > suffering a silent truncation. Or I have not seen a report of it. > > *** Decisions *** > > Now we have some decisions to make. > > Do we: > > (1) just check-in the patch? > (2) check-in the patch and release a 5.6.4? > > Memory errors make me nervous, but that's a personal position. > However, Crypto++ is a security library and I think we should exercise > an abundance of caution, so I'm kind of leaning towards (2). > > *** > > Any thoughts or suggestions? Are there other alternatives we should > consider? > > Jeff > -- > -- > You received this message because you are subscribed to the "Crypto++ > Users" Google Group. > To unsubscribe, send an email to > [email protected]. > More information about Crypto++ and this group is available at > http://www.cryptopp.com. > --- > You received this message because you are subscribed to the Google > Groups "Crypto++ Users" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to [email protected] > <mailto:[email protected]>. > For more options, visit https://groups.google.com/d/optout. -- -- You received this message because you are subscribed to the "Crypto++ Users" Google Group. To unsubscribe, send an email to [email protected]. More information about Crypto++ and this group is available at http://www.cryptopp.com. --- You received this message because you are subscribed to the Google Groups "Crypto++ Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
