On Mon, Oct 19, 2015 at 03:55:09PM +0000, Pascal Cuoq via RT wrote: > > One actual sequence for which the pointers ap and rp end up being identical > is as follows: > > 1/ probable_prime_dh_safe calls BN_sub(q, q, t1) > > 2/ in BN_sub, r and a are then aliases > > 3/ BN_sub calls BN_usub(r, a, b) so r and a are still aliases in BN_usub > > 4/ in BN_usub, ap = a->d; and rp = r->d; > then the 2 pointers can be incremented, but an identical number of times > > 5/ then memcpy is called with rp and ap that are still aliases, which is > undefined behavior
So I've been looking at this, and it seems the patch makes sense at first sight. The manpage says that for BN_add(), BN_mul(), BN_sqr(), BN_mod_mul() and BN_gcd() r can be one of the other BIGNUMs that got passed, but it doesn't say so for BN_sub(). So one could also argue that probable_prime_dh_safe() shouldn't have called BN_sub() like that. But we have various other callers internally that call BN_sub() like that. So we should probably either fix all the callers not to do that, or really make sure that it works properly when they alias and document that they can. And I'm currently in favor of making it safe for them to alias. (It should probably only be allowed to alias a, not b.) But I need to take a closer look to make sure that the patch is enough or that something else needs to be changed too. Kurt _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
