BUG #16070: A double-free bug in interfaces/libpq/fe-secure-openssl.c
The following bug has been logged on the website:
Bug reference: 16070
Logged by: rongxin wu
Email address: wurongxin@xmu.edu.cn
PostgreSQL version: 10.10
Operating system: ubuntu 18
Description:
In
https://github.com/postgres/postgres/blob/REL_10_STABLE/src/interfaces/libpq/fe-secure-openssl.c,
at Line 1206, it would call the function "ENGINE_finish" and free
conn->engine. At Line 1207, it would call the function "ENGINE_free" and
free conn->engine again. This would lead to a double free bug. Please read
the following code snippet.
https://github.com/postgres/postgres/blob/REL_10_STABLE/src/interfaces/libpq/fe-secure-openssl.c
896. static int
897. initialize_SSL(PGconn *conn)
898. {
...
1206. ENGINE_finish(conn->engine);
1207. ENGINE_free(conn->engine);
...
int ENGINE_finish(ENGINE *e)
{
...
to_return = engine_unlocked_finish(e, 1);
...
return to_return;
}
int engine_unlocked_finish(ENGINE *e, int unlock_for_handlers)
{
...
if (!engine_free_util(e, 0)) {
ENGINEerr(ENGINE_F_ENGINE_UNLOCKED_FINISH,
ENGINE_R_FINISH_FAILED);
return 0;
}
return to_return;
}
int engine_free_util(ENGINE *e, int not_locked)
{
...
OPENSSL_free(e); // a free wrapper
return 1;
}
int ENGINE_free(ENGINE *e)
{
return engine_free_util(e, 1);
}
Please help to check. Thanks.
PG Bug reporting form <noreply@postgresql.org> writes:
In
https://github.com/postgres/postgres/blob/REL_10_STABLE/src/interfaces/libpq/fe-secure-openssl.c,
at Line 1206, it would call the function "ENGINE_finish" and free
conn->engine. At Line 1207, it would call the function "ENGINE_free" and
free conn->engine again. This would lead to a double free bug.
I don't really believe this; if there were a double-free problem here,
we'd surely have noticed it long since.
Taking a look at the OpenSSL source code, it looks like engine_free_util
decrements a reference count and doesn't actually delete anything until
that's gone to zero. So maybe the refcount is 2 at the beginning of
this sequence?
regards, tom lane
On Sun, Oct 20, 2019 at 10:17:11AM -0400, Tom Lane wrote:
I don't really believe this; if there were a double-free problem here,
we'd surely have noticed it long since.
Yeah.
Taking a look at the OpenSSL source code, it looks like engine_free_util
decrements a reference count and doesn't actually delete anything until
that's gone to zero. So maybe the refcount is 2 at the beginning of
this sequence?
The docs of OpenSSL mention the use of both successively, where
ENGINE_free() does the cleanup after ENGINE_by_id(), and
ENGINE_finish() cleans up after ENGINE_init():
https://www.openssl.org/docs/man1.1.0/man3/ENGINE_finish.html
And an actual issue is that we have no coverage for it:
https://coverage.postgresql.org/src/interfaces/libpq/fe-secure-openssl.c.gcov.html
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
The docs of OpenSSL mention the use of both successively, where
ENGINE_free() does the cleanup after ENGINE_by_id(), and
ENGINE_finish() cleans up after ENGINE_init():
https://www.openssl.org/docs/man1.1.0/man3/ENGINE_finish.html
Yeah, that reference page pretty definitely agrees with what
we're doing.
And an actual issue is that we have no coverage for it:
https://coverage.postgresql.org/src/interfaces/libpq/fe-secure-openssl.c.gcov.html
Oh, hmm ... I'd supposed that the code in question was exercised
in normal SSL connections, but now I see it's not so. It looks
like you need a non-default SSL "engine" to be available?? Might
be hard to test this as a routine thing if it requires additional
software.
regards, tom lane