amx_Allot() returns success when it shouldn't
#1

There's a bug in amx_Allot() where it checks whether there's enough space on the heap:

Код:
if (amx->stk - amx->hea - cells*sizeof(cell) < STKMARGIN)
  return AMX_ERR_MEMORY;
The expression on the left is always positive because of the convertion to size_t (which is unsigned). It can be fixed by replacing the first line with:

Код:
if ((size_t)amx->stk < (size_t)(amx->hea + cells*sizeof(cell) + STKMARGIN))
This also affects amx_PushString() and amx_PushArray() as they use this function internally.
Reply
#2

How it affects to plugins? to streamer?
Reply
#3

Quote:
Originally Posted by SPAWN_METAL
Посмотреть сообщение
How it affects to plugins? to streamer?
Streamer is OK as it doesn't use these functions. GDK 3.3+ based plugins are fine too as I fixed this internally. Can't say about other plugins, the ones that pass strings or arrays to callbacks are probably affected by this.

Even the server itself can suffer from it - I saw at least one crash report related to this bug. It's pretty easy to trigger:

pawn Код:
#include <a_http>
#include <a_samp>

forward MyHttpResponse(index, response_code, data[]);

main() {
    HTTP(1, HTTP_GET, "www.******.com/search?q=atata", "", "MyHttpResponse");
}

public MyHttpResponse(index, response_code, data[]) {
    printf("Reponse was %d", response_code);
}
The page is clearly bigger than 4 KB, so amx_PushString() will try to write past the top of the stack because it'll think it's OK as amx_Allot() returned no error, which would obviously result in a bad memory access.
Reply
#4

Why wasn't this fixed in 0.3z?
Reply
#5

Quote:
Originally Posted by xeeZ
Посмотреть сообщение
Why wasn't this fixed in 0.3z?
Oh, so that was the reason why my server crashed sometimes with the crashdetect natives...
I think we have to wait a year again for the next SA:MP release :/
Reply
#6

I guess it's the same story as with CallLocal/RemoteFunction - the empty string bug has been there for ages, but Kalcor doesn't seem to care. Perhaps he would argue that it's a feature rather than a bug.
Reply
#7

Since everything is working for everyone I would not expect any fix for that, but well done for finding it.
Reply
#8

Can we please see something done about this in 0.3.7?
Reply


Forum Jump:


Users browsing this thread: 1 Guest(s)