Bug #68276 [Asn]: Reproducible memory corruption: pgsql conflicts with openssl extension

Edit report at https://bugs.php.net/bug.php?id=68276&edit=1 ID: 68276 Updated by: yohgaki@php.net Reported by: dmitry dot koterov at gmail dot com Summary: Reproducible memory corruption: pgsql conflicts with openssl extension Status: Assigned Type: Bug Package: PostgreSQL related Operating System: Ubuntu 14 PHP Version: 5.5.18 Assigned To: yohgaki Block user comment: N Private report: N New Comment: Thank you William. I'm not following pgsql-hackers list closely. Will the patch be merged to PostgreSQL anytime soon? Previous Comments: ------------------------------------------------------------------------ [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) ------------------------------------------------------------------------ [2015-02-22 04:35:36] william dot welter at 4linux dot com dot br I can reproduce de problem.. I do some debugging and understand the real problem: 1- Firstly the use of PQinitSSL nor PQinitOpenSSL not solve the problem, im already test it. 2 - Calling openssl_pkey_get_public('a') PHP try to evaluate the string as X509 certificate with libcrypto and this throws error. ( see stack: ERR_put_error() at err.c:726 0x7ffff6f206f4 PEM_read_bio() at pem_lib.c:689 0x7ffff6f58586 PEM_bytes_read_bio() at pem_lib.c:281 0x7ffff6f5706f PEM_ASN1_read_bio() at pem_oth.c:78 0x7ffff6f5a9e6 php_openssl_x509_from_zval() at openssl.c:1.283 0x468700 php_openssl_x509_from_zval() at openssl.c:1.226 0x468700 php_openssl_evp_from_zval() at openssl.c:2.864 0x469cd4 zif_openssl_pkey_get_public() at openssl.c:3.310 0x46bc2b ) 3 - The problem is that libcryto/openssl share the same error struct on the entire thread, then libqp will use the same struct and will be aware of the errors: see crypto/err/err.c ERR_STATE *ERR_get_state(void) { static ERR_STATE fallback; ERR_STATE *ret, tmp, *tmpp = NULL; int i; CRYPTO_THREADID tid; err_fns_check(); CRYPTO_THREADID_current(&tid); CRYPTO_THREADID_cpy(&tmp.tid, &tid); ret = ERRFN(thread_get_item) (&tmp); /* ret == the error state, if NULL, make a new one */ if (ret == NULL) { ret = (ERR_STATE *)OPENSSL_malloc(sizeof(ERR_STATE)); if (ret == NULL) return (&fallback); CRYPTO_THREADID_cpy(&ret->tid, &tid); ret->top = 0; ret->bottom = 0; for (i = 0; i < ERR_NUM_ERRORS; i++) { ret->err_data[i] = NULL; ret->err_data_flags[i] = 0; } tmpp = ERRFN(thread_set_item) (ret); /* To check if insertion failed, do a get. */ if (ERRFN(thread_get_item) (ret) != ret) { ERR_STATE_free(ret); /* could not insert it */ return (&fallback); } /* * If a race occured in this function and we came second, tmpp is the * first one that we just replaced. */ if (tmpp) ERR_STATE_free(tmpp); } return ret; } 4 - This error only appear with 160K because with this size the multiplier with the buffersize over-read the length of data available and this make the return of "SSL_read()" be "-1" which triggers "SSL_get_error" function to return error. On normal conditions "SSL_get_error" should return called SSL_ERROR_WANT_READ that is normal and libpq can handle this. But in this case (with previous errors stored on the error struct) "ERR_peek_error" function called in the "SSL_get_error" will get the first error occurred (no start line) which leads to return SSL_ERROR_SSL that is handle as a fatal error by libpq. fe-secure.c pqsecure_read(PGconn *conn, void *ptr, size_t len) { ssize_t n; int result_errno = 0; char sebuf[256]; #ifdef USE_SSL if (conn->ssl) { int err; DECLARE_SIGPIPE_INFO(spinfo); /* SSL_read can write to the socket, so we need to disable SIGPIPE */ DISABLE_SIGPIPE(conn, spinfo, return -1); rloop: SOCK_ERRNO_SET(0); n = SSL_read(conn->ssl, ptr, len); err = SSL_get_error(conn->ssl, n); switch (err) { case SSL_ERROR_NONE: if (n < 0) { /* Not supposed to happen, so we don't translate the msg */ printfPQExpBuffer(&conn->errorMessage, "SSL_read failed but did not provide error information\n"); /* assume the connection is broken */ result_errno = ECONNRESET; } break; case SSL_ERROR_WANT_READ: n = 0; break; case SSL_ERROR_WANT_WRITE: /* * Returning 0 here would cause caller to wait for read-ready, * which is not correct since what SSL wants is wait for * write-ready. The former could get us stuck in an infinite * wait, so don't risk it; busy-loop instead. */ goto rloop; case SSL_ERROR_SYSCALL: if (n < 0) { result_errno = SOCK_ERRNO; REMEMBER_EPIPE(spinfo, result_errno == EPIPE); if (result_errno == EPIPE || result_errno == ECONNRESET) printfPQExpBuffer(&conn->errorMessage, libpq_gettext( "server closed the connection unexpectedly\n" "\tThis probably means the server terminated abnormally\n" "\tbefore or while processing the request.\n")); else printfPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL SYSCALL error: %s\n"), SOCK_STRERROR(result_errno, sebuf, sizeof(sebuf))); } else { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL SYSCALL error: EOF detected\n")); /* assume the connection is broken */ result_errno = ECONNRESET; n = -1; } break; case SSL_ERROR_SSL: { char *errm = SSLerrmessage(); printfPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL error: %s\n"), errm); SSLerrfree(errm); /* assume the connection is broken */ result_errno = ECONNRESET; n = -1; break; } case SSL_ERROR_ZERO_RETURN: /* * Per OpenSSL documentation, this error code is only returned * for a clean connection closure, so we should not report it * as a server crash. */ printfPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL connection has been closed unexpectedly\n")); result_errno = ECONNRESET; n = -1; break; default: printfPQExpBuffer(&conn->errorMessage, libpq_gettext("unrecognized SSL error code: %d\n"), err); /* assume the connection is broken */ result_errno = ECONNRESET; n = -1; break; } RESTORE_SIGPIPE(conn, spinfo); } else #endif /* USE_SSL */ { n = recv(conn->sock, ptr, len, 0); if (n < 0) { result_errno = SOCK_ERRNO; /* Set error message if appropriate */ switch (result_errno) { #ifdef EAGAIN case EAGAIN: #endif #if defined(EWOULDBLOCK) && (!defined(EAGAIN) || (EWOULDBLOCK != EAGAIN)) case EWOULDBLOCK: #endif case EINTR: /* no error message, caller is expected to retry */ break; #ifdef ECONNRESET case ECONNRESET: printfPQExpBuffer(&conn->errorMessage, libpq_gettext( "server closed the connection unexpectedly\n" "\tThis probably means the server terminated abnormally\n" "\tbefore or while processing the request.\n")); break; #endif default: printfPQExpBuffer(&conn->errorMessage, libpq_gettext("could not receive data from server: %s\n"), SOCK_STRERROR(result_errno, sebuf, sizeof(sebuf))); break; } } } /* ensure we return the intended errno to caller */ SOCK_ERRNO_SET(result_errno); return n; } ssl/ssl_lib.c int SSL_get_error(const SSL *s, int i) { int reason; unsigned long l; BIO *bio; if (i > 0) return (SSL_ERROR_NONE); /* * Make things return SSL_ERROR_SYSCALL when doing SSL_do_handshake etc, * where we do encode the error */ if ((l = ERR_peek_error()) != 0) { if (ERR_GET_LIB(l) == ERR_LIB_SYS) return (SSL_ERROR_SYSCALL); else return (SSL_ERROR_SSL); } if ((i < 0) && SSL_want_read(s)) { bio = SSL_get_rbio(s); if (BIO_should_read(bio)) return (SSL_ERROR_WANT_READ); ... Another way to see that the shared error struct is the problem: <?php // su - postgres -c psql // # create database test; // # create role test with password 'test' login; // Also be sure that ssl=true is set in postgresql.conf! $sql = "SELECT repeat('a', 160000)"; $c = pg_connect("host='127.0.0.1' dbname='test' user='test' password='test' sslmode='require'"); $r = openssl_pkey_get_public('a'); while ($msg = openssl_error_string()) echo $msg . "<br />\n"; if ($r = pg_query($sql)) echo "OK\n"; else echo pg_last_error($c); Im trying to solve this problem, possible solutions: 1 - Find a way to not share the error struct with the entire thread on PHP 2 - Clear the error struct before send querys on libpq 3 - function ERR_peek_error() get the last error instead of the first error on OpenSSL ------------------------------------------------------------------------ [2014-10-21 14:38:46] dmitry dot koterov at gmail dot com Description: ------------ Long story short: pgsql extension does not call libpg's PQinitSSL nor PQinitOpenSSL inside itself, though it is REQUIRED by libpq if OpenSSL is used elsewhere (e.g. in "openssl" PHP extension). When openssl & pgsql extensions are used in the same script, php process sometimes crashes with a coredump (rarely reproducible). The core file shows an access violation in pgsql extension (at "efree(notice->message);" line), but the bug is caused NOT by this code, it is caused by a deep memory corruption which influences the above code (one of such influences). I've found a 100% reproducible case for this bug, but with a bit different result. See the test script below: if you run it in a x86_64 machine, PHP 5.3 or 5.4+, PostgreSQL 9.1 or 9.3+ (at least), you see a strange error message. If you play with the test buffer size (decrease it from 160K to e.g. 10K), the problem disappears. If no openssl extension is used (just remove the call to "openssl_pkey_get_public"), the problem disappears as well. Why do I suspect that the problem is in the pgsql extension code? Because pgsql extension does not call PQinitSSL nor PQinitOpenSSL from libpg, but libpq requires these calls if it is used in an environment which already uses OpenSSL library: according to http://www.postgresql.org/docs/9.1/static/libpq-ssl.html "If your application initializes libssl and/or libcrypto libraries and libpq is built with SSL support, you should call PQinitOpenSSL to tell libpq that the libssl and/or libcrypto libraries have been initialized by your application, so that libpq will not also initialize those libraries." P.S. Here is the same-looking note from 2009: http://linux.m2osw.com/postgresql_openssl_conflict Test script: --------------- <?php // su - postgres -c psql // # create database test; // # create role test with password 'test' login; // Also be sure that ssl=true is set in postgresql.conf! $sql = "SELECT repeat('a', 160000)"; $c = pg_connect("host='127.0.0.1' dbname='test' user='test' password='test' sslmode='require'"); $r = openssl_pkey_get_public('a'); if ($r = pg_query($sql)) echo "OK\n"; else echo pg_last_error($c); Expected result: ---------------- OK Actual result: -------------- PHP Warning: pg_query(): Query failed: SSL error: no start line in /root/a.php on line 8 Sometimes (very-very rare) it crashes the php5-fpm process, see a backtrace for e.g. PHP 5.3 below (I did not test the crash itself at PHP 5.4+, but I tested the "SSL error: no start line" in all PHP versions, and the roots of these two behaviors are same). Note that there is nothing wrong with notice->message, it's a memory corruption happened long time before, and seems this memory corruption spreads over multiple requests to the same php-fpm process, because the crash is reproduced more frequently when I run the test script above than when I do not run it at all. Program terminated with signal 11, Segmentation fault. #0 0x00000000006ba1e3 in _zend_mm_free_canary_int (heap=0x2519320, p=0x73d7f11b0c14b99c) at /build/buildd/php5-5.3.10/Zend/zend_alloc_canary.c:2086 2086 /build/buildd/php5-5.3.10/Zend/zend_alloc_canary.c: No such file or directory. (gdb) bt #0 0x00000000006ba1e3 in _zend_mm_free_canary_int (heap=0x2519320, p=0x73d7f11b0c14b99c) at /build/buildd/php5-5.3.10/Zend/zend_alloc_canary.c:2086 #1 0x00007f8e033a5231 in _php_pgsql_notice_ptr_dtor (ptr=0x2519320) at /build/buildd/php5-5.3.10/ext/pgsql/pgsql.c:835 #2 0x00000000006a7ed0 in zend_hash_clean (ht=0x7f8e035bb168) at /build/buildd/php5-5.3.10/Zend/zend_hash.c:761 #3 0x00007f8e033a9dc0 in zm_deactivate_pgsql (type=38900512, module_number=202684828) at /build/buildd/php5-5.3.10/ext/pgsql/pgsql.c:1034 #4 0x00000000006a132c in module_registry_cleanup (module=0x2519320) at /build/buildd/php5-5.3.10/Zend/zend_API.c:2168 #5 0x00000000006a82ac in zend_hash_reverse_apply (ht=0xde83e0, apply_func=0x6a1310 <module_registry_cleanup>) at /build/buildd/php5-5.3.10/Zend/zend_hash.c:959 #6 0x000000000069a0f0 in zend_deactivate_modules () at /build/buildd/php5-5.3.10/Zend/zend.c:939 #7 0x0000000000647105 in php_request_shutdown (dummy=0x2519320) at /build/buildd/php5-5.3.10/main/main.c:1638 #8 0x000000000042b71c in main (argc=41420936, argv=0x2780628) at /build/buildd/php5-5.3.10/sapi/fpm/fpm/fpm_main.c:1913 (gdb) up #1 0x00007f8e033a5231 in _php_pgsql_notice_ptr_dtor (ptr=0x2519320) at /build/buildd/php5-5.3.10/ext/pgsql/pgsql.c:835 835 efree(notice->message); ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=68276&edit=1