Skip to content

Critical: check for Size overflow when using palloc #2408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
sfc-gh-dachristensen wants to merge 1 commit into from
+10 −0
Closed

Critical: check for Size overflow when using palloc #2408

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/backend/utils/adt/agtype_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,6 +1023,8 @@ static agtype_value *push_agtype_value_scalar(agtype_parse_state **pstate,
{
(*pstate)->size = 4;
}
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_value))
ereport(ERROR, "allocation request exceeds size limits");
(*pstate)->cont_val.val.array.elems =
palloc(sizeof(agtype_value) * (*pstate)->size);
Comment on lines +1026 to 1029
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ereport is being called with a raw string here, but in this codebase ereport expects an error-reporting clause (e.g., errmsg(...), optionally with errcode(...)). As written, this won’t compile (and it also deviates from the nearby ERRCODE_PROGRAM_LIMIT_EXCEEDED usage). Update these new overflow checks to use the standard ereport(ERROR, (errmsg(...))) form (and consider an appropriate errcode).

Copilot uses AI. Check for mistakes.
(*pstate)->last_updated_value = NULL;
Expand All @@ -1034,6 +1036,8 @@ static agtype_value *push_agtype_value_scalar(agtype_parse_state **pstate,
(*pstate)->cont_val.type = AGTV_OBJECT;
(*pstate)->cont_val.val.object.num_pairs = 0;
(*pstate)->size = 4;
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_pair))
ereport(ERROR, "allocation request exceeds size limits");
(*pstate)->cont_val.val.object.pairs =
palloc(sizeof(agtype_pair) * (*pstate)->size);
Comment on lines +1039 to 1042
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above: ereport(ERROR, "...") is not a valid ereport invocation in this file (see existing patterns like ereport(ERROR, (errmsg(...)))). This will fail to compile unless rewritten to the standard ereport(ERROR, (...)) form.

Copilot uses AI. Check for mistakes.
(*pstate)->last_updated_value = NULL;
Expand Down Expand Up @@ -1120,6 +1124,8 @@ static void append_key(agtype_parse_state *pstate, agtype_value *string)
if (object->val.object.num_pairs >= pstate->size)
{
pstate->size *= 2;
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_pair))
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In append_key, pstate is an agtype_parse_state *, so (*pstate)->size is invalid (compile error). This should reference the struct directly (consistent with the surrounding code using pstate->size).

Suggested change
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_pair))
if (pstate->size > SIZE_MAX / sizeof(agtype_pair))

Copilot uses AI. Check for mistakes.
ereport(ERROR, "allocation request exceeds size limits");
Comment on lines 1126 to +1128
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overflow check is placed after pstate->size *= 2. If Size wraps on multiplication, pstate->size can become small and pass the subsequent SIZE_MAX / sizeof(...) check, reintroducing the exact under-allocation risk this PR is trying to fix. The check needs to happen before doubling (or use a safe arithmetic helper) so that overflow in the growth step itself is prevented.

Suggested change
pstate->size *= 2;
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_pair))
ereport(ERROR, "allocation request exceeds size limits");
if (pstate->size > SIZE_MAX / 2 ||
pstate->size > (SIZE_MAX / sizeof(agtype_pair)) / 2)
ereport(ERROR, "allocation request exceeds size limits");
pstate->size *= 2;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The newly added ereport in this block uses a raw string literal. In this file, ereport must be invoked with an error clause list (e.g., errmsg(...)), otherwise it won’t compile. Please rewrite this ereport to match the existing patterns in the surrounding code.

Suggested change
ereport(ERROR, "allocation request exceeds size limits");
ereport(ERROR,
(errmsg("allocation request exceeds size limits")));

Copilot uses AI. Check for mistakes.
object->val.object.pairs = repalloc(
object->val.object.pairs, sizeof(agtype_pair) * pstate->size);
}
Expand Down Expand Up @@ -1167,6 +1173,8 @@ static void append_element(agtype_parse_state *pstate,
if (array->val.array.num_elems >= pstate->size)
{
pstate->size *= 2;
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_value))
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In append_element, pstate is an agtype_parse_state *, so (*pstate)->size is invalid and will not compile. Use the direct field access form already used elsewhere in this function (pstate->size).

Suggested change
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_value))
if (pstate->size > SIZE_MAX / sizeof(agtype_value))

Copilot uses AI. Check for mistakes.
ereport(ERROR, "allocation request exceeds size limits");
Comment on lines 1175 to +1177
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same pattern as the object growth path: the overflow check comes after pstate->size *= 2, so an overflow in the doubling step itself can wrap size and evade the subsequent SIZE_MAX / sizeof(...) guard. Move the guard before the doubling (or use safe-arithmetic helpers) to ensure capacity growth can’t wrap and cause an under-sized repalloc.

Suggested change
pstate->size *= 2;
if ((*pstate)->size > SIZE_MAX / sizeof(agtype_value))
ereport(ERROR, "allocation request exceeds size limits");
size_t new_size;
if (pstate->size > SIZE_MAX / 2)
ereport(ERROR, "allocation request exceeds size limits");
new_size = pstate->size * 2;
if (new_size > SIZE_MAX / sizeof(agtype_value))
ereport(ERROR, "allocation request exceeds size limits");
pstate->size = new_size;

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ereport added here is using a raw string literal, but ereport in this codebase expects an errmsg(...) (and optionally errcode(...)) clause list; as written, it will not compile. Update this call to follow the existing ereport(ERROR, (errmsg(...))) style in the file.

Suggested change
ereport(ERROR, "allocation request exceeds size limits");
ereport(ERROR, (errmsg("allocation request exceeds size limits")));

Copilot uses AI. Check for mistakes.
array->val.array.elems = repalloc(array->val.array.elems,
sizeof(agtype_value) * pstate->size);
}
Expand Down Expand Up @@ -1631,6 +1639,8 @@ bool agtype_deep_contains(agtype_iterator **val,
uint32 j = 0;

/* Make room for all possible values */
if (num_lhs_elems > SIZE_MAX / sizeof(agtype_value))
ereport(ERROR, "allocation request exceeds size limits");
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new guard uses ereport(ERROR, "..."), which doesn’t match the valid ereport(ERROR, (errmsg(...))) style used elsewhere in this file and will not compile as-is. Update it to the standard ereport(ERROR, (...)) form so the overflow check is actually effective.

Suggested change
ereport(ERROR, "allocation request exceeds size limits");
ereport(ERROR,
(errmsg("allocation request exceeds size limits")));

Copilot uses AI. Check for mistakes.
lhs_conts = palloc(sizeof(agtype_value) * num_lhs_elems);

for (i = 0; i < num_lhs_elems; i++)
Expand Down
Loading