Skip to content

MDEV-39767: include name pool size in dynamic column get bounds check #5129

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

Open
jmestwa-coder wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −2
Open

MDEV-39767: include name pool size in dynamic column get bounds check #5129

Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion mysys/ma_dyncol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,7 @@ find_column(DYN_HEADER *hdr, uint numkey, LEX_STRING *strkey)
char nmkeybuff[DYNCOL_NUM_CHAR]; /* to fit max 2 bytes number */
DBUG_ASSERT(hdr->header != NULL);

if (hdr->header + hdr->header_size > hdr->data_end)
if (hdr->header + hdr->header_size + hdr->nmpool_size > hdr->data_end)
return TRUE;
Comment on lines +2104 to 2105
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 26, 2026

Choose a reason for hiding this comment

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

security-high high

The pointer addition hdr->header + hdr->header_size + hdr->nmpool_size can lead to pointer wrapping or integer overflow if hdr->header_size and hdr->nmpool_size are extremely large or maliciously crafted. This could bypass the bounds check and result in an out-of-bounds read. To prevent this, perform the bounds check by comparing the sizes against the remaining buffer space without performing additions that could overflow.

  if (hdr->header_size > (size_t)(hdr->data_end - hdr->header) ||
      hdr->nmpool_size > (size_t)(hdr->data_end - hdr->header) - hdr->header_size)
    return TRUE;

Copy link
Copy Markdown
Member

@gkodinov gkodinov May 27, 2026

Choose a reason for hiding this comment

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

I'd second Gemini here. It's better to be safe.


/* fix key */
Expand Down
30 changes: 29 additions & 1 deletion unittest/mysys/ma_dyncol-t.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,13 +735,40 @@ static void test_mdev_9773()
mariadb_dyncol_free(&dynstr);
}

static void test_named_format_oob_nmpool()
{
/*
Named-format record whose declared name pool does not fit into the
record. The read path must reject it instead of letting the name
pointer run past the record buffer.
*/
DYNAMIC_COLUMN str;
DYNAMIC_COLUMN_VALUE res;
int rc;
uchar buff[9]=
{
0x04, /* flags: named format, offset size 2 */
0x01, 0x00, /* column count = 1 */
0x01, 0x00, /* name pool size = 1 (does not fit the record) */
0x00, 0x00, /* entry: name offset = 0 */
0x00, 0x00 /* entry: combined data offset and type */
};
bzero(&str, sizeof(str));
str.str= (char*) buff;
str.length= sizeof(buff);

rc= mariadb_dyncol_get_num(&str, 1, &res);
ok(rc != ER_DYNCOL_OK || res.type == DYN_COL_NULL,
"%s", "named name pool past record end");
}

int main(int argc __attribute__((unused)), char **argv)
{
uint i;
char *big_string= (char *)malloc(1024*1024);

MY_INIT(argv[0]);
plan(68);
plan(69);

if (!big_string)
exit(1);
Expand Down Expand Up @@ -875,6 +902,7 @@ int main(int argc __attribute__((unused)), char **argv)
test_mdev_4994();
test_mdev_4995();
test_mdev_9773();
test_named_format_oob_nmpool();

my_end(0);
return exit_status();
Expand Down