# refcount problems created by openssl_public_encrypt(symbol table corruption and double free)

- Thread: https://externals.io/message/16295
- Original archive: https://news-web.php.net/php.internals/16295
- Started by Kamesh Jayachandran on 2005-05-25T14:14:24+00:00
- Messages: 3
- Reply by email to internals@lists.php.net
- More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt

---

## #16295 — Kamesh Jayachandran — 2005-05-25T14:14:24+00:00

URL: https://externals.io/message/16295#16295

Hi Wez & Jani,
Following snippet causes double free of memory, corrupts the symbol
table.
<?php
$pk=false;//As openssl_get_publickey($nonsense) can give false
openssl_public_encrypt("Test",$encrypted,$pk);
$pk=false;
?>

The culprit is php_openssl_evp_from_zval which is called by
openssl_public_encrypt.
openssl_public_encrypt reduces the refcount of $pk from 2 to 1. 
zend_ptr_stack_clear_multiple reduces it again to 0 as a normal cleanup
upon return from openssl_public_encrypt.
And hence zval associated with $pk is getting freed.
But symbol table still refers to freed pointer.

The patch for 5.0 Tree is available at,
http://puggy.symonds.net/~kameshj/openssl.c.patch.5.0

With regards
Kamesh Jayachandran

---

## #16296 — Wez Furlong — 2005-05-25T19:20:25+00:00

URL: https://externals.io/message/16295#16296 · In reply to: #16295

The patch doesn't look quite right.

-       convert_to_string_ex(val);
+       zval tmpz;
+       zval *tmpzp;
+       tmpz = *(*val);
+       zval_copy_ctor(&tmpz);
+       tmpz.refcount=1;
+       tmpzp = &tmpz;

I think that this is a place where convert_to_string(val) should be
used instead.

-                   in = BIO_new_mem_buf(Z_STRVAL_PP(val), Z_STRLEN_PP(val));
+                   in = BIO_new_mem_buf(Z_STRVAL_PP(&tmpzp),
Z_STRLEN_PP(&tmpzp));

If you still need to use zval* instead of zval**, you can use
Z_STRVAL_P(tmpzp) instead of taking the address and using
Z_STRVAL_PP().

Can you check your patch again using convert_to_string() instead?

--Wez.


> Hi Wez & Jani,
> Following snippet causes double free of memory, corrupts the symbol
> table.
> <?php
> $pk=false;//As openssl_get_publickey($nonsense) can give false
> openssl_public_encrypt("Test",$encrypted,$pk);
> $pk=false;
> ?>
> 
> The culprit is php_openssl_evp_from_zval which is called by
> openssl_public_encrypt.
> openssl_public_encrypt reduces the refcount of $pk from 2 to 1.
> zend_ptr_stack_clear_multiple reduces it again to 0 as a normal cleanup
> upon return from openssl_public_encrypt.
> And hence zval associated with $pk is getting freed.
> But symbol table still refers to freed pointer.
> 
> The patch for 5.0 Tree is available at,
> http://puggy.symonds.net/~kameshj/openssl.c.patch.5.0
> 
> With regards
> Kamesh Jayachandran

---

## #16297 — Kamesh Jayachandran — 2005-05-26T07:29:01+00:00

URL: https://externals.io/message/16295#16297 · In reply to: #16296

Thanks Wez for pointing out.
New patch is available at,
http://puggy.symonds.net/~kameshj/openssl.c.patch.5.0
convert_to_string_ex is needed but not on val but on &tmpzp.(Got lost
while manually copying the changes from my NetWare build environment to
Linux).

With regards
Kamesh Jayachandran
On Wed, 25 May 2005 15:20:25 -0400, "Wez Furlong" <kingwez@gmail.com>
said:
> The patch doesn't look quite right.
> 
> -       convert_to_string_ex(val);
> +       zval tmpz;
> +       zval *tmpzp;
> +       tmpz = *(*val);
> +       zval_copy_ctor(&tmpz);
> +       tmpz.refcount=1;
> +       tmpzp = &tmpz;
> 
> I think that this is a place where convert_to_string(val) should be
> used instead.
> 
> -                   in = BIO_new_mem_buf(Z_STRVAL_PP(val),
> Z_STRLEN_PP(val));
> +                   in = BIO_new_mem_buf(Z_STRVAL_PP(&tmpzp),
> Z_STRLEN_PP(&tmpzp));
> 
> If you still need to use zval* instead of zval**, you can use
> Z_STRVAL_P(tmpzp) instead of taking the address and using
> Z_STRVAL_PP().
> 
> Can you check your patch again using convert_to_string() instead?
> 
> --Wez.
> 
> 
> > […]