Re: Missing NULL check after calling ecpg_strdup

On Mon, Jul 21, 2025 at 11:48:27AM +0300, Aleksander Alekseev wrote:
> + /* Check for NULL to prevent segfault */
> + if (con->name != NULL && strcmp(connection_name, con->name) == 0)
> break;

I have spent some time rechecking the whole code, and I have
backpatched this part. One pattern where I've easily been able to
trigger the problem is by specifying "\0" as the database name. The
preprocessor rejects empty strings, but it fails to reject the case of
a CONNECT TO "\0". Perhaps it should actually, found that funny..
Anyway, it does not change the fact that if we do a named connection
lookup in a stack where at least one NULL connection is set, we would
crash during the lookup.

I have also hacked on the rest, spotted a couple of bugs and
inconsistencies.

- stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
+ stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno, &alloc_failed);
+ if (alloc_failed)
+ return false;

Hmm.. Aren't you missing a va_end(args) in the early exit you are
adding here?

In ecpg_store_input() and ecpg_do_prologue(), we can do without any
alloc_failed. All the inputs can never be NULL.

I think that the order of the operations in ecpg_auto_prepare() is
incorrect: we would add statements to the cache even if the strdup()
fails. We have only one caller of ecpg_auto_prepare(), where the
*name we set does not matter.

At the end, I finish with the attached, where alloc_failed matters for
the failure checks with repeated calls of strdup() in ECPGconnect()
and also the setlocale() case. This is for HEAD due to how unlikely
these issues would occur in practice.
--
Michael