Re: [openssl-dev] [openssl.org #4155] In function int_thread_del_item, when hash == int_thread_hash, one is passed to free and the other is used in a comparison

2015-12-23 Thread Kurt Roeckx via RT
On Mon, Nov 30, 2015 at 08:12:58PM +, Kurt Roeckx via RT wrote:
> On Tue, Nov 24, 2015 at 11:06:44AM +, Pascal Cuoq via RT wrote:
> > This issue is similar in nature to 4151 
> > (http://www.mail-archive.com/openssl-dev@openssl.org/msg40950.html ): it is 
> > about a dangling pointer being used, but not used for dereferencing, so 
> > it's not a memory error. The dangling pointer is used in a comparison.
> > 
> > The function int_thread_del_item can reach the point were it calls 
> > "lh_ERR_STATE_free(int_thread_hash);" with hash == int_thread_hash. That 
> > attached patch prints a message like "hash == int_thread_hash == 0xb2a6d0" 
> > when this happens. Just after the call to lh_ERR_STATE_free, both hash and 
> > int_thread_hash contain dangling pointers. The variable int_thread_hash is 
> > immediately set to NULL.
> > 
> > The problem that I am reporting is that just afterwards,  is passed to 
> > the function int_thread_release:
> > 
> > https://github.com/openssl/openssl/blob/079a1a9014b89661f0a612a5a9724ad9c77f21a3/crypto/err/err.c#L412
> > 
> > In that function, the argument "hash" points to the local variable "hash" 
> > of int_thread_del_item (which contains a dangling pointer).
> > Thus the comparison "*hash == NULL" involves a dangling pointer:
> 
> I think the following untested patch should fix that:

The patch has been applied.


Kurt


___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


Re: [openssl-dev] [openssl.org #4155] In function int_thread_del_item, when hash == int_thread_hash, one is passed to free and the other is used in a comparison

2015-11-30 Thread Kaduk, Ben via RT
On 11/24/2015 05:06 AM, Pascal Cuoq via RT wrote:
> This issue is similar in nature to 4151 
> (http://www.mail-archive.com/openssl-dev@openssl.org/msg40950.html ): it is 
> about a dangling pointer being used, but not used for dereferencing, so it's 
> not a memory error. The dangling pointer is used in a comparison.
>
> The function int_thread_del_item can reach the point were it calls 
> “lh_ERR_STATE_free(int_thread_hash);” with hash == int_thread_hash. That 
> attached patch prints a message like “hash == int_thread_hash == 0xb2a6d0” 
> when this happens. Just after the call to lh_ERR_STATE_free, both hash and 
> int_thread_hash contain dangling pointers. The variable int_thread_hash is 
> immediately set to NULL.
>
> The problem that I am reporting is that just afterwards,  is passed to 
> the function int_thread_release:
>
> https://github.com/openssl/openssl/blob/079a1a9014b89661f0a612a5a9724ad9c77f21a3/crypto/err/err.c#L412
>
>

Note that per J.2 of C99 (n1256.pdf page 503, document-internal
numbering), using the value of a pointer that refers to space
deallocated by a call to free is undefined behavior, even for just
comparison.  (This is covered in passing at
http://web.torek.net/torek/c/numbers2.html)

-Ben


___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev


[openssl-dev] [openssl.org #4155] In function int_thread_del_item, when hash == int_thread_hash, one is passed to free and the other is used in a comparison

2015-11-24 Thread Pascal Cuoq via RT
This issue is similar in nature to 4151 
(http://www.mail-archive.com/openssl-dev@openssl.org/msg40950.html ): it is 
about a dangling pointer being used, but not used for dereferencing, so it's 
not a memory error. The dangling pointer is used in a comparison.

The function int_thread_del_item can reach the point were it calls 
“lh_ERR_STATE_free(int_thread_hash);” with hash == int_thread_hash. That 
attached patch prints a message like “hash == int_thread_hash == 0xb2a6d0” when 
this happens. Just after the call to lh_ERR_STATE_free, both hash and 
int_thread_hash contain dangling pointers. The variable int_thread_hash is 
immediately set to NULL.

The problem that I am reporting is that just afterwards,  is passed to the 
function int_thread_release:

https://github.com/openssl/openssl/blob/079a1a9014b89661f0a612a5a9724ad9c77f21a3/crypto/err/err.c#L412

In that function, the argument “hash” points to the local variable “hash” of 
int_thread_del_item (which contains a dangling pointer).
Thus the comparison “*hash == NULL” involves a dangling pointer:

https://github.com/openssl/openssl/blob/079a1a9014b89661f0a612a5a9724ad9c77f21a3/crypto/err/err.c#L343

With the attached patch, executing test/enginetest reproduces the problem for 
me:

$ ./enginetest

enginetest beginning
…
Tests completed happily
hash == int_thread_hash == 0x1a3f6d0
Now using *hash (0x1a3f6d0) in a comparison




show_pointers.patch
Description: Binary data
___
openssl-bugs-mod mailing list
openssl-bugs-...@openssl.org
https://mta.openssl.org/mailman/listinfo/openssl-bugs-mod___
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev