Skip to content

postfix: Decrement blr length after reading value from blr in parse_format for blr_int128 - #9124

Open
TreeHunter9 wants to merge 1 commit into
FirebirdSQL:masterfrom
TreeHunter9:master_postfix_blr_length_validation
Open

postfix: Decrement blr length after reading value from blr in parse_format for blr_int128#9124
TreeHunter9 wants to merge 1 commit into
FirebirdSQL:masterfrom
TreeHunter9:master_postfix_blr_length_validation

Conversation

@TreeHunter9

Copy link
Copy Markdown
Contributor

No description provided.

@TreeHunter9 TreeHunter9 changed the title Postfix: Decrement blr length after reading value from blr in parse_format for blr_int128 postfix: Decrement blr length after reading value from blr in parse_format for blr_int128 Aug 13, 2026
@aafemt

aafemt commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Perhaps, there is also a problem in PARSE_messages: blr_length is not decremented on consumption of message number, so parse_format receives value one bigger than actual blr length.

@TreeHunter9

Copy link
Copy Markdown
Contributor Author

Perhaps, there is also a problem in PARSE_messages: blr_length is not decremented on consumption of message number, so parse_format receives value one bigger than actual blr length.

As I can see, blr_length is decremented before reading blr value:

		if (blr_length-- == 0) // <-- here
		{
			error = true;
			break;
		}

		const USHORT msg_number = *blr++;

Or were you referring to some other place?

@aafemt

aafemt commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

This decrement is paired with consumption of blr_message verb one line above.

Then msg_number is consumed, but corresponding decrement of blr_length is only 14 lines below:

==>		const USHORT msg_number = *blr++;

		rem_fmt* const format = parse_format(blr, blr_length);
		if (!format)
		{
			error = true;
			break;
		}

		RMessage* next = FB_NEW RMessage(format->fmt_length);
		next->msg_next = message;
		message = next;
		message->msg_address = reinterpret_cast<UCHAR*>(format);
		message->msg_number = msg_number;

==>		if (blr_length-- == 0)
		{
			error = true;
			break;
		}

@TreeHunter9

Copy link
Copy Markdown
Contributor Author

This decrement is paired with consumption of blr_message verb one line above.

Hm, I though it is paired differently:

	if (blr_length < 3)
		return NULL;
	blr_length -= 3;  // <--- We need to read 3 u8 values from blr --->

	const SSHORT version = *blr++;  // <--- 1 --->
	if (version != blr_version4 && version != blr_version5)
		return NULL;

	if (*blr++ != blr_begin)  // <--- 2 --->
		return NULL;

	RMessage* message = NULL;

	bool error = false;

	while (*blr++ == blr_message)  // <--- 3 --->
	{
		if (blr_length-- == 0)  // <--- We need to read 1 u8 values from blr --->
		{
			error = true;
			break;
		}

		const USHORT msg_number = *blr++;  // <--- 1 --->

		rem_fmt* const format = parse_format(blr, blr_length);
		if (!format)
		{
			error = true;
			break;
		}

		RMessage* next = FB_NEW RMessage(format->fmt_length);
		next->msg_next = message;
		message = next;
		message->msg_address = reinterpret_cast<UCHAR*>(format);
		message->msg_number = msg_number;

		if (blr_length-- == 0)  // <--- It checks the next `while (*blr++ == blr_message)` iteration --->
		{
			error = true;
			break;
		}
	}

@aafemt

aafemt commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Ah, you are right, I'm sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants