Zeex / sampgdk

Write SA-MP gamemodes in C/C++
http://zeex.github.io/sampgdk
Apache License 2.0
153 stars 83 forks source link

Possible out-of-bounds read when calling sa-mp natives which have 'char *' arguments #176

Closed grasmanek94 closed 7 years ago

grasmanek94 commented 7 years ago

When calling natives that write back to a string (char array) out-of-bounds reads can occur, or even access violations/segfaults.

Take for example the following code from the 4.5.2 amalgamation release:

SAMPGDK_NATIVE(int, GetPlayerName(int playerid, char * name, int size)) {
  static AMX_NATIVE native;
  cell retval;
  cell params[4];
  cell name_;
  sampgdk_log_debug("GetPlayerName(%d, \"%s\", %d)", playerid, name, size);
  native = sampgdk_native_find_flexible("GetPlayerName", native);
  sampgdk_fakeamx_push(size, &name_);
  params[0] = 3 * sizeof(cell);
  params[1] = (cell)playerid;
  params[2] = name_;
  params[3] = (cell)size;
  retval = native(sampgdk_fakeamx_amx(), params);
  sampgdk_fakeamx_get_string(name_, name, size);
  sampgdk_fakeamx_pop(name_);
  return (int)(retval);
}

When the caller issues the following from either C++ or C with the following code:

void break_things()
{
    char name[32];
    GetPlayerName(0, name, sizeof(name));
    printf("%s", name);
}

This code could possibly crash at:

sampgdk_log_debug("GetPlayerName(%d, \"%s\", %d)", playerid, name, size);

because the name variable is uninitialized. This means that the %s parameter in the log function could read past the name variable which will result in undefined behavior (probably segfaults/access violations)

Zeex commented 7 years ago

Fixed, thanks.