Fix use validator on stack after returning.

[buriedpot]

Describe the bug

pg_documentdb/src/metadata/collection.c:1905, the function ParseAndGetValidatorSpec returns a variable validator. It points to a memory area allocated on stack.

//...
	EnsureTopLevelFieldType(validatorName, iter, BSON_TYPE_DOCUMENT);
	const bson_value_t *validator = bson_iter_value(iter);

	/* Large and overly complex validation rules can impact database performance, especially during write operations. */
	if (validator->value.v_doc.data_len > (uint32_t) MaxSchemaValidatorSize)
//...

pg_documentdb/src/commands/create_collection_view.c:382, the function ParseCreateSpec assign the pointer to spec->validator, which will be used after the function returning.

else if (strcmp(key, "validator") == 0)
{
	spec->validator = ParseAndGetValidatorSpec(&createIter, "create.validator",
										   hasSchemaValidationSpec);
}

Environment

• Linux
• PostgreSQL 16
• Architecture

Reproduction Steps

compilation: gcc -fstack-protector-strong -O0
run test case: schema_validation_insert, schema_validation

postgres=# SELECT documentdb_api.create_collection_view('schema_validation', '{ "create": "col", "validator": {"$jsonSchema": {"bsonType": "object", "properties": {"a": {"bsonType": "int"}}}}, "validationLevel": "strict", "validationAction": "error"}');
NOTICE:  creating collection
ERROR:  expected a document to create a bson object

Solution

Use palloc0. It's worth mentioning that the validator->value.v_doc.data is allocated on heap, so assigning the value of validator to spec->validator is enough and we don't need to do deep copying.

[safern]

Could we avoid the palloc0 changing ParseAndGetValidatorSpec to take a pointer and assign the value to such pointer?

Then we could call the function like:
ParseAndGetValidatorSpec(&createIter, "create.validator", hasSchemaValidationSpec, &spec->validator)

[buriedpot]

Thank you for your review 😊

Do you mean that we should still allocate validator on stack or we should allocate validator on heap in function ParseAndGetValidatorSpec instead of ParseCreateSpec?

As you can see at create_collection_view.c:154, after function ParseCreateSpec returning, spec->validator will be still used, so I think that using palloc is necessary. I can put the palloc into function ParseAndGetValidatorSpec.

[buriedpot]

Or you want to use palloc instead of palloc0?

I think using palloc is indeed more reasonable.

[safern]

What I mean is that spec in create_collection_view is palloc'd already and not in the stack. So we could just change the signature of the ParseAndGetValidatorSpec function to accept a bson_value_t * instead of returning bson_value_t.

Then it will populate the bson_value_t * passed in with the parsed validator spec.

Then we call is like this:

ParseAndGetValidatorSpec(&createIter, "create.validator", hasSchemaValidationSpec, &spec->validator)

[buriedpot]

Thank you for your advice, I will try to make some modifications.

[buriedpot]

Thank you for your review.
The validator field in spec is a pointer, so palloc is inevitable.

I can put palloc in function ParseAndGetValidatorSpec instead of ParseCreateSpec.

The core problem is that the function bson_iter_value will return a pointer which points to a memory area on stack.

[visridha]

bson_value_t is a struct right? - so if we have spec's validator be a bson_value_t (instead of a const bson_value_t *) then when calling ParseAndGetValidatorSpec as called above, we can have something like

specValidator = *bson_iter_value(&iterator)

[buriedpot]

Ok, I will make some modifications. Thank you for reviewing.

[safern]

Hi, @buriedpot -- where you able to do the adjustment suggested here?

[safern]

@buriedpot friendly ping, will you be able to finish this? Thank you in advance.

[buriedpot]

Thank you for ping. Unfortunately, I currently don't have time to modify this fix. If someone else is willing to take over this PR, feel free to use my work as a starting point. May I close this issue? Best regards.

…

---Original--- From: "Santiago Fernandez ***@***.***> Date: Sat, May 10, 2025 00:39 AM To: ***@***.***>; Cc: ***@***.******@***.***>; Subject: Re: [microsoft/documentdb] Fix use validator on stack afterreturning. (PR #91) safern left a comment (microsoft/documentdb#91) @buriedpot friendly ping, will you be able to finish this? Thank you in advance. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

[safern]

@buriedpot thanks for your reply. We will try to finish your work then.