Edit report at https://bugs.php.net/bug.php?id=68276&edit=1
ID: 68276
Comment by: dz at heroku dot com
Reported by: dmitry dot koterov at gmail dot com
Summary: Reproducible memory corruption: pgsql conflicts with
openssl extension
Status: Analyzed
Type: Bug
Package: OpenSSL related
Operating System: Ubuntu 14
PHP Version: 5.5.18
Block user comment: N
Private report: N
New Comment:
We created a fix for Postgres: http://www.postgresql.org/message-id/flat/CAM3SWZSOJ1p-6jE+h8iii6WboBmyFHuJto=S2Fk==y1wLV3pSQ@mail.gmail.com
Latest point releases were two days ago, and it did not make it in, so hopefully next time...
Previous Comments:
------------------------------------------------------------------------
[2015-04-02 01:08:02] william dot welter at 4linux dot com dot br
The following patch is my proposal to solve the problem on PHP side, by not left a landmine on
openssl error queue for other applications, without broke currently "openssl_error_string"
behaviour.
This consist of create an internal ssl error context on PHP, and clean de original ssl error queue
after store on the internal queue.
diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
index c64f1e92..787d513 100644
--- a/ext/openssl/openssl.c
+++ b/ext/openssl/openssl.c
@@ -498,6 +498,11 @@ static int le_x509;
static int le_csr;
static int ssl_stream_data_index;
+
+
+
+
+
int php_openssl_get_x509_list_id(void) /* {{{ */
{
return le_x509;
@@ -808,6 +813,52 @@ static int add_oid_section(struct php_x509_request * req TSRMLS_DC) /* {{{ */
static const EVP_CIPHER * php_openssl_get_evp_cipher_from_algo(long algo);
+
+
+
+static PHP_SSL_ERROR_CONTEXT *ssl_error_context;
+
+void php_openssl_store_errors(void)
+{
+ PHP_SSL_ERROR_QUEUE *err,*tmp;
+ char buf[512];
+ unsigned long val;
+
+ //initialize error context if is null;
+ if(ssl_error_context==NULL) {
+ ssl_error_context = emalloc(sizeof(PHP_SSL_ERROR_CONTEXT));
+ ssl_error_context->current=NULL;
+ }
+
+ err = emalloc(sizeof(PHP_SSL_ERROR_QUEUE));
+ err->next=NULL;
+
+ // Retrieve error from openssl error queue
+ val = ERR_get_error();
+ if (val) {
+ ERR_error_string(val, buf);
+ err->err_str=emalloc(strlen(buf));
+ strcpy(err->err_str,buf);
+ ERR_clear_error();
+ }else{
+ return ;
+ }
+
+ //If error queue is empty, create the first
+ if(ssl_error_context->current==NULL){
+ ssl_error_context->current=err;
+ }else{
+ //Else, append error to last element
+ tmp=ssl_error_context->current;
+ while(tmp->next!=NULL)
+ {
+ tmp=tmp->next;
+ }
+ tmp->next=err;
+ }
+
+}
+
static int php_openssl_parse_config(struct php_x509_request * req, zval * optional_args TSRMLS_DC)
/* {{{ */
{
char * str;
@@ -3323,6 +3374,7 @@ PHP_FUNCTION(openssl_pkey_get_public)
pkey = php_openssl_evp_from_zval(cert, 1, NULL, 1, &Z_LVAL_P(return_value) TSRMLS_CC);
if (pkey == NULL) {
+ php_openssl_store_errors();
RETURN_FALSE;
}
zend_list_addref(Z_LVAL_P(return_value));
@@ -4204,19 +4256,36 @@ PHP_FUNCTION(openssl_public_decrypt)
Returns a description of the last error, and alters the index of the error messages. Returns
false when the are no more messages */
PHP_FUNCTION(openssl_error_string)
{
- char buf[512];
- unsigned long val;
+ PHP_SSL_ERROR_QUEUE *tmp;
+ char *err_str;
if (zend_parse_parameters_none() == FAILURE) {
- return;
+ return;
}
- val = ERR_get_error();
- if (val) {
- RETURN_STRING(ERR_error_string(val, buf), 1);
- } else {
- RETURN_FALSE;
+ // Return false if error queue is empty
+ if(ssl_error_context->current==NULL){
+ RETURN_FALSE;;
}
+
+ // Get first error
+ tmp=ssl_error_context->current;
+
+ //Store error msg
+ err_str=emalloc(strlen(tmp->err_str));
+ strcpy(err_str,tmp->err_str);
+
+ //Update references of the error queue
+ if(tmp->next!=NULL){
+ ssl_error_context->current=tmp->next;
+ }else{
+ ssl_error_context->current=NULL;
+ }
+
+ //Free memory of the current error
+ efree(tmp);
+ RETURN_STRING(err_str,1);
+
}
/* }}} */
diff --git a/ext/openssl/php_openssl.h b/ext/openssl/php_openssl.h
index 86d83b7..a3d9b67 100644
--- a/ext/openssl/php_openssl.h
+++ b/ext/openssl/php_openssl.h
@@ -86,6 +86,15 @@ PHP_FUNCTION(openssl_csr_get_public_key);
#endif
#endif
+typedef struct php_ssl_error{
+ char *err_str;
+ struct php_ssl_error* next;
+}PHP_SSL_ERROR_QUEUE ;
+
+
+typedef struct php_ssl_error_context{
+ struct php_ssl_error* current;
+}PHP_SSL_ERROR_CONTEXT;
/*
* Local variables:
Of course, i need to call php_openssl_store_errors on every "false" return of every
function on the openssl extension.
I will put this as pull request on github.
------------------------------------------------------------------------
[2015-03-06 17:37:37] william dot welter at 4linux dot com dot br
The bug that i open on the libpq revealed that libpq has an internal control to not leave error on
queue always "poping" the last element if an error occur after each call.
The problem is that PHP leaves error on the queue if the developer don't call
openssl_error_string() for each ssl call. This behaviour makes a landmine for every lib that use
OpenSSL that not clean the error queue before IO calls.
I propose a patch to clean the error queue on libpq before the IO operations, but this can't be
applied because on current versions because this can break currently applications. I will propose a
patch for next major release.
On my opinion PHP should not leave errors on error queue, because this can break other libs that use
OpenSSL. I will work on patch to create an internal struct to store the errors to keep the OpenSSL
error queue clean to other libs.
------------------------------------------------------------------------
[2015-02-24 04:41:56] yohgaki@php.net
Thank you William.
I'm not following pgsql-hackers list closely.
Will the patch be merged to PostgreSQL anytime soon?
------------------------------------------------------------------------
[2015-02-24 04:39:06] yohgaki@php.net
Assign myself to remind me.
------------------------------------------------------------------------
[2015-02-24 03:18:54] william dot welter at 4linux dot com dot br
On my opinion the bug is not on PHP, is on libpq pgsecure_read()/pqsecure_write() functions that not
clean error queue before IO operations as described on OpenSSL manual
(https://www.openssl.org/docs/ssl/SSL_get_error.html#DESCRIPTION).
Im already open a bug on PostgreSQL community with patch suggested
(http://www.postgresql.org/message-id/20150224030956.2529.83279@wrigleys.postgresql.org)
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=68276
--
Edit this bug report at https://bugs.php.net/bug.php?id=68276&edit=1