Sanitize load_amxscript() and add load_amxscript_ex/MF_LoadAmxScriptEx() requiring error max length (#530)

* Add a saner version of load_amxscript and use SafeStrcpy/Sprintf

* Reflect the change in core

* Add LoadAmxScriptEx API function

* Reflect the change in CSX

* Reflect the change in DodX

* Reflect the change in TFCX

* Reflect the change in TSX

* Add few comments
This commit is contained in:
Vincent Herbet 2018-08-30 18:49:42 +02:00 committed by GitHub
parent 76378fd5d0
commit 6e9947b64f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 66 additions and 47 deletions

View File

@ -18,14 +18,14 @@
extern const char *no_function;
CPluginMngr::CPlugin* CPluginMngr::loadPlugin(const char* path, const char* name, char* error, int debug)
CPluginMngr::CPlugin* CPluginMngr::loadPlugin(const char* path, const char* name, char* error, size_t maxLength, int debug)
{
CPlugin** a = &head;
while (*a)
a = &(*a)->next;
*a = new CPlugin(pCounter++, path, name, error, debug);
*a = new CPlugin(pCounter++, path, name, error, maxLength, debug);
return (*a);
}
@ -137,7 +137,7 @@ int CPluginMngr::loadPluginsFromFile(const char* filename, bool warn)
continue;
}
CPlugin* plugin = loadPlugin(pluginsDir, pluginName, error, debugFlag);
CPlugin* plugin = loadPlugin(pluginsDir, pluginName, error, sizeof(error), debugFlag);
if (plugin->getStatusCode() == ps_bad_load)
{
@ -267,7 +267,7 @@ const char* CPluginMngr::CPlugin::getStatus() const
return "error";
}
CPluginMngr::CPlugin::CPlugin(int i, const char* p, const char* n, char* e, int d) : name(n), title(n), m_pNullStringOfs(nullptr), m_pNullVectorOfs(nullptr)
CPluginMngr::CPlugin::CPlugin(int i, const char* p, const char* n, char* e, size_t m, int d) : name(n), title(n), m_pNullStringOfs(nullptr), m_pNullVectorOfs(nullptr)
{
const char* unk = "unknown";
@ -280,7 +280,7 @@ CPluginMngr::CPlugin::CPlugin(int i, const char* p, const char* n, char* e, int
char* path = build_pathname_r(file, sizeof(file), "%s/%s", p, n);
code = 0;
memset(&amx, 0, sizeof(AMX));
int err = load_amxscript(&amx, &code, path, e, d);
int err = load_amxscript_ex(&amx, &code, path, e, m, d);
if (err == AMX_ERR_NONE)
{

View File

@ -66,7 +66,7 @@ public:
CPlugin* next;
int id;
CPlugin(int i, const char* p, const char* n, char* e, int d);
CPlugin(int i, const char* p, const char* n, char* e, size_t m, int d);
~CPlugin();
bool m_Debug;
@ -122,7 +122,7 @@ public:
// Interface
CPlugin* loadPlugin(const char* path, const char* name, char* error, int debug);
CPlugin* loadPlugin(const char* path, const char* name, char* error, size_t maxLength, int debug);
void unloadPlugin(CPlugin** a);
int loadPluginsFromFile(const char* filename, bool warn=true);

View File

@ -284,6 +284,7 @@ extern "C" size_t get_amxstring_r(AMX *amx, cell amx_addr, char *destination, in
int amxstring_len(cell* cstr);
int load_amxscript(AMX* amx, void** program, const char* path, char error[64], int debug);
int load_amxscript_ex(AMX* amx, void** program, const char* path, char *error, size_t maxLength, int debug);
int set_amxnatives(AMX* amx, char error[64]);
int set_amxstring(AMX *amx, cell amx_addr, const char *source, int max);
int set_amxstring_simple(cell *dest, const char *source, int max);

View File

@ -108,7 +108,7 @@ static binlogfuncs_t logfuncs =
};
#endif
int load_amxscript(AMX *amx, void **program, const char *filename, char error[64], int debug)
int load_amxscript_internal(AMX *amx, void **program, const char *filename, char *error, size_t maxLength, int debug)
{
*error = 0;
size_t bufSize;
@ -127,7 +127,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
if (!*program)
{
strcpy(error, "Failed to allocate memory");
ke::SafeStrcpy(error, maxLength, "Failed to allocate memory");
return (amx->error = AMX_ERR_MEMORY);
}
@ -140,31 +140,31 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
case CAmxxReader::Err_None:
break;
case CAmxxReader::Err_FileOpen:
strcpy(error, "Plugin file open error");
ke::SafeStrcpy(error, maxLength, "Plugin file open error");
return (amx->error = AMX_ERR_NOTFOUND);
case CAmxxReader::Err_FileRead:
strcpy(error, "Plugin file read error");
ke::SafeStrcpy(error, maxLength, "Plugin file read error");
return (amx->error = AMX_ERR_NOTFOUND);
case CAmxxReader::Err_InvalidParam:
strcpy(error, "Internal error: Invalid parameter");
ke::SafeStrcpy(error, maxLength, "Internal error: Invalid parameter");
return (amx->error = AMX_ERR_NOTFOUND);
case CAmxxReader::Err_FileInvalid:
strcpy(error, "Invalid Plugin");
ke::SafeStrcpy(error, maxLength, "Invalid Plugin");
return (amx->error = AMX_ERR_FORMAT);
case CAmxxReader::Err_SectionNotFound:
strcpy(error, "Searched section not found (.amxx)");
ke::SafeStrcpy(error, maxLength, "Searched section not found (.amxx)");
return (amx->error = AMX_ERR_NOTFOUND);
case CAmxxReader::Err_DecompressorInit:
strcpy(error, "Decompressor initialization failed");
ke::SafeStrcpy(error, maxLength, "Decompressor initialization failed");
return (amx->error = AMX_ERR_INIT);
case CAmxxReader::Err_Decompress:
strcpy(error, "Internal error: Decompress");
ke::SafeStrcpy(error, maxLength, "Internal error: Decompress");
return (amx->error = AMX_ERR_NOTFOUND);
case CAmxxReader::Err_OldFile:
strcpy(error, "Plugin uses deprecated format. Update compiler");
ke::SafeStrcpy(error, maxLength, "Plugin uses deprecated format. Update compiler");
return (amx->error = AMX_ERR_FORMAT);
default:
strcpy(error, "Unknown error");
ke::SafeStrcpy(error, maxLength, "Unknown error");
return (amx->error = AMX_ERR_NOTFOUND);
}
} else {
@ -178,7 +178,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
if (magic != AMX_MAGIC)
{
strcpy(error, "Invalid Plugin");
ke::SafeStrcpy(error, maxLength, "Invalid Plugin");
return (amx->error = AMX_ERR_FORMAT);
}
@ -191,7 +191,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
{
if ((hdr->file_version < CUR_FILE_VERSION))
{
sprintf(error, "Plugin needs newer debug version info");
ke::SafeStrcpy(error, maxLength, "Plugin needs newer debug version info");
return (amx->error = AMX_ERR_VERSION);
}
else if ((hdr->flags & AMX_FLAG_DEBUG) != 0)
@ -209,13 +209,13 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
{
dbg_FreeInfo(pDbg);
delete pDbg;
sprintf(error, "Debug loading error %d", err);
ke::SafeSprintf(error, maxLength, "Debug loading error %d", err);
return (amx->error = AMX_ERR_INIT);
}
amx->flags |= AMX_FLAG_DEBUG;
} else {
sprintf(error, "Plugin not compiled with debug option");
ke::SafeStrcpy(error, maxLength, "Plugin not compiled with debug option");
return (amx->error = AMX_ERR_INIT);
}
} else {
@ -238,7 +238,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
delete pDbg;
}
sprintf(error, "Load error %d (invalid file format or version)", err);
ke::SafeSprintf(error, maxLength, "Load error %d (invalid file format or version)", err);
return (amx->error = AMX_ERR_INIT);
}
@ -276,7 +276,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
{
delete[] np;
delete[] rt;
strcpy(error, "Failed to initialize JIT'd plugin");
ke::SafeStrcpy(error, maxLength, "Failed to initialize JIT'd plugin");
return (amx->error = AMX_ERR_INIT);
}
@ -307,14 +307,14 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
if (*program == 0)
{
strcpy(error, "Failed to allocate memory");
ke::SafeStrcpy(error, maxLength, "Failed to allocate memory");
return (amx->error = AMX_ERR_MEMORY);
}
} else {
delete[] np;
delete[] rt;
sprintf(error, "Failed to initialize plugin (%d)", err);
ke::SafeSprintf(error, maxLength, "Failed to initialize plugin (%d)", err);
return (amx->error = AMX_ERR_INIT_JIT);
}
@ -325,7 +325,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
if (!script)
{
ke::SafeSprintf(error, 64, "Failed to allocate memory for script");
ke::SafeStrcpy(error, maxLength, "Failed to allocate memory for script");
return (amx->error = AMX_ERR_MEMORY);
}
@ -341,7 +341,7 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
{
if (amx_Register(amx, core_Natives, -1) != AMX_ERR_NONE)
{
sprintf(error, "Plugin uses an unknown function (name \"%s\") - check your modules.ini.", no_function);
ke::SafeSprintf(error, maxLength, "Plugin uses an unknown function (name \"%s\") - check your modules.ini.", no_function);
return (amx->error = AMX_ERR_NOTFOUND);
}
} else {
@ -352,6 +352,17 @@ int load_amxscript(AMX *amx, void **program, const char *filename, char error[64
return (amx->error = AMX_ERR_NONE);
}
int load_amxscript_ex(AMX *amx, void **program, const char *filename, char *error, size_t maxLength, int debug)
{
return load_amxscript_internal(amx, program, filename, error, maxLength, debug);
}
// Deprecated. Use load_amxscript_ex() or MF_LoadAmxScriptEx() for modules. This function is kept to maintain backward compatibility.
int load_amxscript(AMX *amx, void **program, const char *filename, char error[64], int debug)
{
return load_amxscript_internal(amx, program, filename, error, 64 /* error max length */, debug);
}
const char *StrCaseStr(const char *as, const char *bs)
{
static char a[256];
@ -1759,9 +1770,10 @@ void Module_CacheFunctions()
REGISTER_FUNC("GetAmxScriptName", MNF_GetAmxScriptName)
REGISTER_FUNC("FindAmxScriptByName", MNF_FindAmxScriptByName)
REGISTER_FUNC("FindAmxScriptByAmx", MNF_FindAmxScriptByAmx)
REGISTER_FUNC("LoadAmxScript", load_amxscript)
REGISTER_FUNC("LoadAmxScript", load_amxscript) // Deprecated. Please use LoadAmxScriptEx instead.
REGISTER_FUNC("LoadAmxScriptEx", load_amxscript_ex)
REGISTER_FUNC("UnloadAmxScript", unload_amxscript)
// String / mem in amx scripts support
REGISTER_FUNC("SetAmxString", set_amxstring)
REGISTER_FUNC("SetAmxStringUTF8Char", set_amxstring_utf8_char)

View File

@ -139,9 +139,9 @@ void RankSystem::clear(){
bool RankSystem::loadCalc(const char* filename, char* error)
bool RankSystem::loadCalc(const char* filename, char* error, size_t maxLength)
{
if ((MF_LoadAmxScript(&calc.amx,&calc.code,filename,error,0)!=AMX_ERR_NONE)||
if ((MF_LoadAmxScriptEx(&calc.amx,&calc.code,filename, error, maxLength, 0)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 11 , &calc.amxAddr1, &calc.physAddr1)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr2, &calc.physAddr2)!=AMX_ERR_NONE)||
(MF_AmxFindPublic(&calc.amx,"get_score",&calc.func)!=AMX_ERR_NONE)){

View File

@ -110,7 +110,7 @@ public:
void saveRank( const char* filename );
void loadRank( const char* filename );
RankStats* findEntryInRank(const char* unique, const char* name, bool isip=false);
bool loadCalc(const char* filename, char* error);
bool loadCalc(const char* filename, char* error, size_t maxLength);
inline int getRankNum( ) const { return rankNum; }
void clear();
void unloadCalc();

View File

@ -421,7 +421,7 @@ void OnAmxxAttach(){
if ( path && *path )
{
char error[128];
g_rank.loadCalc( MF_BuildPathname("%s",path) , error );
g_rank.loadCalc( MF_BuildPathname("%s",path) , error, sizeof(error));
}
if ( !g_rank.begin() )

View File

@ -128,9 +128,9 @@ void RankSystem::clear(){
}
}
bool RankSystem::loadCalc(const char* filename, char* error)
bool RankSystem::loadCalc(const char* filename, char* error, size_t maxLength)
{
if ((MF_LoadAmxScript(&calc.amx,&calc.code,filename,error,0)!=AMX_ERR_NONE)||
if ((MF_LoadAmxScriptEx(&calc.amx,&calc.code,filename, error, maxLength, 0)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr1, &calc.physAddr1)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr2, &calc.physAddr2)!=AMX_ERR_NONE)||
(MF_AmxFindPublic(&calc.amx,"get_score",&calc.func)!=AMX_ERR_NONE)){

View File

@ -104,7 +104,7 @@ public:
void saveRank( const char* filename );
void loadRank( const char* filename );
RankStats* findEntryInRank(const char* unique, const char* name , bool isip = false);
bool loadCalc(const char* filename, char* error);
bool loadCalc(const char* filename, char* error, size_t maxLength);
inline int getRankNum( ) const { return rankNum; }
void clear();
void unloadCalc();

View File

@ -471,7 +471,7 @@ void OnAmxxAttach()
if ( path && *path )
{
char error[128];
g_rank.loadCalc( MF_BuildPathname("%s",path) , error );
g_rank.loadCalc( MF_BuildPathname("%s",path) , error, sizeof(error));
}
if ( !g_rank.begin() )

View File

@ -127,9 +127,9 @@ void RankSystem::clear(){
}
}
bool RankSystem::loadCalc(const char* filename, char* error)
bool RankSystem::loadCalc(const char* filename, char* error, size_t maxLength)
{
if ((MF_LoadAmxScript(&calc.amx,&calc.code,filename,error,0)!=AMX_ERR_NONE)||
if ((MF_LoadAmxScriptEx(&calc.amx,&calc.code,filename, error, maxLength, 0)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr1, &calc.physAddr1)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr2, &calc.physAddr2)!=AMX_ERR_NONE)||
(MF_AmxFindPublic(&calc.amx,"get_score",&calc.func)!=AMX_ERR_NONE)){

View File

@ -102,7 +102,7 @@ public:
void saveRank( const char* filename );
void loadRank( const char* filename );
RankStats* findEntryInRank(const char* unique, const char* name , bool isip = false);
bool loadCalc(const char* filename, char* error);
bool loadCalc(const char* filename, char* error, size_t maxLength);
inline int getRankNum( ) const { return rankNum; }
void clear();
void unloadCalc();

View File

@ -330,7 +330,7 @@ void OnAmxxAttach() {
if ( path && *path )
{
char error[128];
g_rank.loadCalc( MF_BuildPathname("%s",path) , error );
g_rank.loadCalc( MF_BuildPathname("%s",path), error, sizeof(error));
}
if ( !g_rank.begin() )
{

View File

@ -127,9 +127,9 @@ void RankSystem::clear(){
}
}
bool RankSystem::loadCalc(const char* filename, char* error)
bool RankSystem::loadCalc(const char* filename, char* error, size_t maxLength)
{
if ((MF_LoadAmxScript(&calc.amx,&calc.code,filename,error,0)!=AMX_ERR_NONE)||
if ((MF_LoadAmxScriptEx(&calc.amx,&calc.code,filename, error, maxLength, 0)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr1, &calc.physAddr1)!=AMX_ERR_NONE)||
(MF_AmxAllot(&calc.amx, 8 , &calc.amxAddr2, &calc.physAddr2)!=AMX_ERR_NONE)||
(MF_AmxFindPublic(&calc.amx,"get_score",&calc.func)!=AMX_ERR_NONE)){

View File

@ -102,7 +102,7 @@ public:
void saveRank( const char* filename );
void loadRank( const char* filename );
RankStats* findEntryInRank(const char* unique, const char* name , bool isip = false );
bool loadCalc(const char* filename, char* error);
bool loadCalc(const char* filename, char* error, size_t maxLength);
inline int getRankNum( ) const { return rankNum; }
void clear();
void unloadCalc();

View File

@ -344,7 +344,7 @@ void OnAmxxAttach()
if ( path && *path )
{
char error[128];
g_rank.loadCalc( MF_BuildPathname("%s",path) , error );
g_rank.loadCalc( MF_BuildPathname("%s",path), error, sizeof(error));
}
if ( !g_rank.begin() )
{

View File

@ -2399,6 +2399,7 @@ PFN_AMX_EXECV g_fn_AmxExecv;
PFN_AMX_ALLOT g_fn_AmxAllot;
PFN_AMX_FINDPUBLIC g_fn_AmxFindPublic;
PFN_LOAD_AMXSCRIPT g_fn_LoadAmxScript;
PFN_LOAD_AMXSCRIPT_EX g_fn_LoadAmxScriptEx;
PFN_UNLOAD_AMXSCRIPT g_fn_UnloadAmxScript;
PFN_REAL_TO_CELL g_fn_RealToCell;
PFN_CELL_TO_REAL g_fn_CellToReal;
@ -2491,7 +2492,8 @@ C_DLLEXPORT int AMXX_Attach(PFN_REQ_FNPTR reqFnptrFunc)
REQFUNC("GetAmxScript", g_fn_GetAmxScript, PFN_GET_AMXSCRIPT);
REQFUNC("FindAmxScriptByAmx", g_fn_FindAmxScriptByAmx, PFN_FIND_AMXSCRIPT_BYAMX);
REQFUNC("FindAmxScriptByName", g_fn_FindAmxScriptByName, PFN_FIND_AMXSCRIPT_BYNAME);
REQFUNC("LoadAmxScript", g_fn_LoadAmxScript, PFN_LOAD_AMXSCRIPT);
REQFUNC("LoadAmxScript", g_fn_LoadAmxScript, PFN_LOAD_AMXSCRIPT); // Deprecated. Please use LoadAmxScriptEx instead.
REQFUNC("LoadAmxScriptEx", g_fn_LoadAmxScriptEx, PFN_LOAD_AMXSCRIPT_EX);
REQFUNC("UnloadAmxScript", g_fn_UnloadAmxScript, PFN_UNLOAD_AMXSCRIPT);
REQFUNC("GetAmxScriptName", g_fn_GetAmxScriptName, PFN_GET_AMXSCRIPTNAME);
@ -2695,6 +2697,7 @@ void ValidateMacros_DontCallThis_Smiley()
MF_AmxFindPublic(0, 0, 0);
MF_AmxAllot(0, 0, 0, 0);
MF_LoadAmxScript(0, 0, 0, 0, 0);
MF_LoadAmxScriptEx(0, 0, 0, 0, 0, 0);
MF_UnloadAmxScript(0, 0);
MF_RegisterSPForward(0, 0, 0, 0, 0, 0);
MF_RegisterSPForwardByName(0, 0, 0, 0, 0, 0);

View File

@ -2199,6 +2199,7 @@ typedef int (*PFN_AMX_ALLOT) (AMX* /*amx*/, int /*length*/, cell* /*amx_ad
typedef int (*PFN_AMX_FINDPUBLIC) (AMX* /*amx*/, const char* /*func name*/, int* /*index*/);
typedef int (*PFN_AMX_FINDNATIVE) (AMX* /*amx*/, const char* /*func name*/, int* /*index*/);
typedef int (*PFN_LOAD_AMXSCRIPT) (AMX* /*amx*/, void** /*code*/, const char* /*path*/, char[64] /*error info*/, int /* debug */);
typedef int (*PFN_LOAD_AMXSCRIPT_EX) (AMX* /*amx*/, void** /*code*/, const char* /*path*/, char* /*error info*/, size_t /* max length */, int /* debug */);
typedef int (*PFN_UNLOAD_AMXSCRIPT) (AMX* /*amx*/,void** /*code*/);
typedef cell (*PFN_REAL_TO_CELL) (REAL /*x*/);
typedef REAL (*PFN_CELL_TO_REAL) (cell /*x*/);
@ -2274,6 +2275,7 @@ extern PFN_AMX_EXEC g_fn_AmxExec;
extern PFN_AMX_ALLOT g_fn_AmxAllot;
extern PFN_AMX_FINDPUBLIC g_fn_AmxFindPublic;
extern PFN_LOAD_AMXSCRIPT g_fn_LoadAmxScript;
extern PFN_LOAD_AMXSCRIPT_EX g_fn_LoadAmxScriptEx;
extern PFN_UNLOAD_AMXSCRIPT g_fn_UnloadAmxScript;
extern PFN_REAL_TO_CELL g_fn_RealToCell;
extern PFN_CELL_TO_REAL g_fn_CellToReal;
@ -2437,6 +2439,7 @@ void MF_LogError(AMX *amx, int err, const char *fmt, ...);
#define MF_AmxAllot g_fn_AmxAllot
#define MF_AmxFindNative g_fn_AmxFindNative
#define MF_LoadAmxScript g_fn_LoadAmxScript
#define MF_LoadAmxScriptEx g_fn_LoadAmxScriptEx
#define MF_UnloadAmxScript g_fn_UnloadAmxScript
#define MF_MergeDefinitionFile g_fn_MergeDefinition_File
#define amx_ctof g_fn_CellToReal