[HL][TFC] Crash while parsing command menu (menu item length)

[pizzahut2]

Not related to #1730 apart from that it also happens in the command menu.

There have been reports by French and German TFC players experiencing a crash when trying to join a server, with the proposed work-around being to delete the language specific tfc folder ("tfc_french" or "tfc_german") or to change language to English. I was able to reproduce the crash and find out what's causing it.

There is a bug in the current version of TFC which causes it to crash if a menu item is longer than 31 characters. The old version of TFC (steam_legacy) is not affected. The problem probably exists since the October 2024 changes to the client DLL in depot 21 (TFC Base Content).

Work-arounds:

• french_commandmenu.diff.txt
• [HL][TFC] Crash while parsing command menu (menu item length) #3956 (comment)

I removed the single quotes to fit more letters into the maximum of 31 characters. The command string can be longer.

Test case 1
"8" "You finish building the dispenser." "say OK"
A buffer overflow occurs if the original string is too long in the button text.

Test case 2
"8" "#Dispenser_finish" "say OK"
A buffer overflow occurs if the substituted string is too long in the button text.

These are triggered in two different code segments of "CHudTextMessage::LocaliseTextString" in "cl_dll/text_message.cpp".

Proposed bug fix for the client DLL: #3956 (comment)

[pizzahut2]

Work-around for German language: german_commandmenu.diff.txt

No other languages are affected.

[pizzahut2]

I'm looking at cl_dll/vgui_TeamFortressViewport.cpp#L817-L821 and cl_dll/text_message.cpp#L47-L85 . This is the client DLL for Half-Life.

At CHudTextMessage::LocaliseTextString , line 85 it adds a zero behind the current position.

			dst++, src++;
			*dst = 0;

If the source string is longer than the destination, then this means a zero is added behind the buffer (buffer overflow).

Though I don't know if this is actually causing the crash, and also this is the HL client DLL, not the one for TFC.

vgui_TeamFortressViewport.cpp#L817-L821

			cText[31] = '\0';

			// Get the button text
			pfile = gEngfuncs.COM_ParseFile(pfile, token);
			CHudTextMessage::LocaliseTextString( token, cText, sizeof( cText ) );

As a work-around, "sizeof( cText )" could be decreased by one. But the actual bug is in "CHudTextMessage::LocaliseTextString" as described above.

text_message.cpp#L47-L87

char *CHudTextMessage::LocaliseTextString( const char *msg, char *dst_buffer, int buffer_size )
{
	int len = buffer_size;
	char *dst = dst_buffer;
	for ( char *src = (char*)msg; *src != 0 && buffer_size > 0; buffer_size-- )
	{
		if ( *src == '#' )
[...]
		else
		{
			*dst = *src;
			dst++, src++;
			*dst = 0;
		}
	}

The "if" part looks dangerous as well. If a "#" is found, and a corresponding entry in "titles.txt", it copies to the destination without checking if the buffer is long enough.

			// copy string into message over the msg name
			for ( char *wsrc = (char*)clmsg->pMessage; *wsrc != 0; wsrc++, dst++ )
			{
				*dst = *wsrc;
			}
			*dst = 0;

The call to "CHudTextMessage::LocaliseTextString" was added to "vgui_TeamFortressViewport.cpp" with the HL25 SDK Update.

Test case 1
"8" "You finish building the dispenser." "say OK"
A buffer overflow occurs if the original string is too long in the button text.

Test case 2
"8" "#Dispenser_finish" "say OK"
A buffer overflow occurs if the substituted string is too long in the button text.

These are triggered in two different code segments of "CHudTextMessage::LocaliseTextString" in "cl_dll/text_message.cpp".

[pizzahut2]

Proposing a fix for "CHudTextMessage::LocaliseTextString" in "cl_dll/text_message.cpp". It's untested, maybe someone could check if it works? @SamVanheer @kisak-valve @shawns-valve

b1b5cf5...pizzahut2:halflife:patch-1

[0Ky]

In the latest English build version 10210 of Half-Life, if a command menu button text exceeds 32 characters (including the null terminator), the game writes past the end of the fixed size cText[32] buffer, causing a crash. This does not appear to be caused by using non-English characters, but some locale specific builds of TFC, such as German and French, will immediately crash to desktop upon creating or connecting to a server on the Windows build, because the button text simply exceeds the buffer size. I was unable to reproduce this issue on Linux or in Counter-Strike 1.6 on Windows. I have tested the proposed patch to CHudTextMessage::LocaliseTextString, and it appears to fix the crash reliably.