sourcemod icon indicating copy to clipboard operation
sourcemod copied to clipboard

[Bug]: LogError %s format with a large string caused a crash

Open bottiger1 opened this issue 6 months ago • 5 comments

Prerequisites

  • [x] I have checked that my issue doesn't exist yet in the issue tracker

Operating System and Version

ubuntu 22.04 x64

Game / AppID and Version

tf2 440 9742990/24 9742990

SourceMod Version

1.13.0.7234

Metamod:Source Version

No response

Version Verification

  • [ ] I have updated SourceMod to the latest version and the issue persists
  • [ ] I have updated SourceMod to the latest snapshot and the issue persists
  • [ ] I have updated Metamod:Source to the latest snapshot and the issue persists

Updated SourceMod Version

No response

Updated Metamod:Source Version

No response

Description

I passed a very large string to LogError like this, it was probably several thousand bytes as it was a webpage.

It caused a segfault. There should probably be a limit on the string size to prevent a crash.

Program terminated with signal SIGSEGV, Segmentation fault. #0 0xe618da73 in AddString(char**, unsigned int&, char const*, int, int, int) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_deathrun/bin/sourcemod.logic.so [Current thread is 1 (Thread 0xf7b7ecc0 (LWP 1768443))] #0 0xe618da73 in AddString(char**, unsigned int&, char const*, int, int, int) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_deathrun/bin/sourcemod.logic.so #1 0xe618d80f in atcprintf(char*, unsigned int, char const*, SourcePawn::IPluginContext*, int const*, int*) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_deathrun/bin/sourcemod.logic.so #2 0xe62fd792 in SourceModBase::FormatString(char*, unsigned int, SourcePawn::IPluginContext*, int const*, unsigned int) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod/bin/sourcemod.2.tf2.so #3 0xe6146d0d in sm_LogError(SourcePawn::IPluginContext*, int const*) () from /home/tf2/steam/server1/tf/tf/addons/sourcemod_deathrun/bin/sourcemod.logic.so

Steps to Reproduce

Code looked like this. You could probably simulate it with just a large string passed to logerror.

char[] data = new char[size]; Async_CurlGetData(request, data, size); LogError("body: %s", data);

Relevant Log Output


bottiger1 avatar May 31 '25 06:05 bottiger1

The maximum number of characters that LogError can handle in a message is 1024. Including the EOL character, only the first 1023 characters will be formatted, and the rest will be truncated.

https://github.com/alliedmodders/sourcemod/blob/f9929ef9ac163843798a617dd20d3253bd1055e6/core/logic/smn_filesystem.cpp#L916-L923

Test Cases :

#include <sourcemod>

public void OnPluginStart()
{
    char msg[2048];
    for (int i = 0; i < 2047; ++i)
    {
        msg[i] = '0' + (i % 10);
    }
    msg[2047] = '\0';

    LogError(msg);
}

Log output (log message length == 1023) :

L 06/01/2025 - 08:34:55: [test.smx] 012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012

F1F88 avatar May 31 '25 22:05 F1F88

Could you provide more details?

When I tested it with the following code, everything worked fine:

Operating System and Version

Ubuntu 24.04

Game / AppID and Version

NMRIH version 1.14.2

SourceMod Version

1.13.0.7214

Metamod:Source Version

2.0.0-dev+1315

Steps to Reproduce

#include <sourcemod>

#pragma dynamic 65535 + 1024

public void OnPluginStart()
{
    char[] msg = new char[65535];
    for (int i = 0; i < 65534; ++i)
    {
        msg[i] = '0' + (i % 10);
    }
    msg[65534] = '\0';

    LogError("msg: %s", msg);
}

Relevant Log Output

Shell:

sm plugins reload test
L 06/01/2025 - 08:29:32: [test.smx] msg: 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901[SM] Plugin test.smx reloaded successfully.

Error log file:

L 06/01/2025 - 08:29:32: [test.smx] msg: 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567

F1F88 avatar May 31 '25 23:05 F1F88

I have a plugin sending an http request for each player. I reloaded the plugin which caused a flood of requests so it probably caused cloudflare to throw a page error.

Unfortunately I cannot see what the contents was because it crashed when I logged the error as you can see from the crash dump above. All I know is that it was probably several thousand bytes.

If it wasn't due to the size of the string, maybe it is because the string overflowed the stack space and so the string was not null terminated until a really large number or until it hit an invalid memory location. I cannot see any other way it would produce a crash dump like this.

bottiger1 avatar Jun 12 '25 20:06 bottiger1

It looks like the string address is likely pointing to an invalid memory address.

Have you tried using other output methods to confirm that the string is valid?

For example:

F1F88 avatar Jun 13 '25 13:06 F1F88

I don't believe it is possible for the string address to point to invalid memory because it was created through sourcepawn this way (this is copied exactly from my code, its not an example).

char[] data = new char[size];

This is a C++ extension that uses LocalToString to get a pointer to the sourcepawn buffer and then memcpy the data to it. If either was invalid, it would have crashed there.

Async_CurlGetData(request, data, size);

https://github.com/bottiger1/sourcemod-async/blob/cb0a1ce5e5262aedacf37cdb61dbd7e8dc7f14b3/natives.cpp#L122

I've had no crashes since limiting the buffer size to 512.

bottiger1 avatar Jun 13 '25 19:06 bottiger1

In your Changelog it says:

Fixed Async_CurlGetData so it will only add a null byte to the end if the sourcepawn buffer is larger than data received.

Remember that CurlGetData can return any binary data, not just strings, so you should always append a null byte (truncating if necessary) if you intend to blindly treat its output as a string.

Forlix avatar Jun 27 '25 23:06 Forlix

It is fetching a webpage so it wouldn't be binary data.

Oh never mind, I see that I used the function that doesn't null terminate it.

bottiger1 avatar Jun 28 '25 00:06 bottiger1

Unless you're verifying the Content-Type response header, you can't be sure what you're getting back. It might even be a zero-length response body (for example on a redirect), in which case your buffer would be left entirely uninitialized.

Forlix avatar Jun 28 '25 01:06 Forlix