From c851fc9bfbafb152a6a8ed91824475b15f7dd98f Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Wed, 11 Jun 2025 02:11:09 +0200 Subject: [PATCH 01/11] Ensure the squirrel instance for native member function call actually has userpointer type This prevents memory errors when incorrectly invoking native member functions from Squirrel. --- sp/src/vscript/vscript_squirrel.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 93008683..584402a1 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1324,7 +1324,10 @@ SQInteger function_stub(HSQUIRRELVM vm) SQInteger top = sq_gettop(vm); SQUserPointer userptr = nullptr; - sq_getuserpointer(vm, top, &userptr); + if (SQ_FAILED(sq_getuserpointer(vm, top, &userptr))) + { + return sq_throwerror(vm, "Expected userpointer"); + } Assert(userptr); @@ -1425,7 +1428,10 @@ SQInteger function_stub(HSQUIRRELVM vm) if (pFunc->m_flags & SF_MEMBER_FUNC) { SQUserPointer self; - sq_getinstanceup(vm, 1, &self, nullptr); + if (SQ_FAILED(sq_getinstanceup(vm, 1, &self, 0))) + { + return sq_throwerror(vm, "Expected class userpointer"); + } if (!self) { From 681a75a6a708934a85b7acf3dd34d71a04690df6 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Wed, 11 Jun 2025 02:14:13 +0200 Subject: [PATCH 02/11] Check ScriptClassDesc_t of squirrel instance for native member function call This enhances the ScriptFuncDescriptor_t to record the ScriptClassDesc_t of the class for which it gets invoked. The ScriptClassDesc_t are traversed in the function_stub() for native function calls for member functions, to ensure the passed in instance has a compatible type. This prevents memory errors when incorrectly invoking native member functions from Squirrel. --- sp/src/public/vscript/ivscript.h | 4 ++++ sp/src/public/vscript/vscript_templates.h | 2 +- sp/src/vscript/vscript_squirrel.cpp | 22 +++++++++++++++++----- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/sp/src/public/vscript/ivscript.h b/sp/src/public/vscript/ivscript.h index c0a0cf08..1eba404c 100644 --- a/sp/src/public/vscript/ivscript.h +++ b/sp/src/public/vscript/ivscript.h @@ -263,6 +263,8 @@ inline const char * ScriptFieldTypeName( int16 eType) //--------------------------------------------------------- +struct ScriptClassDesc_t; + struct ScriptFuncDescriptor_t { ScriptFuncDescriptor_t() @@ -270,11 +272,13 @@ struct ScriptFuncDescriptor_t m_pszFunction = NULL; m_ReturnType = FIELD_TYPEUNKNOWN; m_pszDescription = NULL; + m_pScriptClassDesc = NULL; } const char *m_pszScriptName; const char *m_pszFunction; const char *m_pszDescription; + ScriptClassDesc_t *m_pScriptClassDesc; ScriptDataType_t m_ReturnType; CUtlVector m_Parameters; }; diff --git a/sp/src/public/vscript/vscript_templates.h b/sp/src/public/vscript/vscript_templates.h index 9f2054cc..bda6bc9e 100644 --- a/sp/src/public/vscript/vscript_templates.h +++ b/sp/src/public/vscript/vscript_templates.h @@ -61,7 +61,7 @@ FUNC_GENERATE_ALL( DEFINE_MEMBER_FUNC_TYPE_DEDUCER ); FUNC_GENERATE_ALL( DEFINE_CONST_MEMBER_FUNC_TYPE_DEDUCER ); -#define ScriptInitMemberFuncDescriptor_( pDesc, class, func, scriptName ) if ( 0 ) {} else { (pDesc)->m_pszScriptName = scriptName; (pDesc)->m_pszFunction = #func; ScriptDeduceFunctionSignature( pDesc, (class *)(0), &class::func ); } +#define ScriptInitMemberFuncDescriptor_( pDesc, class, func, scriptName ) if ( 0 ) {} else { (pDesc)->m_pszScriptName = scriptName; (pDesc)->m_pszFunction = #func; ScriptDeduceFunctionSignature( pDesc, (class *)(0), &class::func ); (pDesc)->m_pScriptClassDesc = GetScriptDesc(nullptr); } #define ScriptInitFuncDescriptorNamed( pDesc, func, scriptName ) if ( 0 ) {} else { (pDesc)->m_pszScriptName = scriptName; (pDesc)->m_pszFunction = #func; ScriptDeduceFunctionSignature( pDesc, &func ); } #define ScriptInitFuncDescriptor( pDesc, func ) ScriptInitFuncDescriptorNamed( pDesc, func, #func ) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 584402a1..39139fec 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1427,18 +1427,30 @@ SQInteger function_stub(HSQUIRRELVM vm) if (pFunc->m_flags & SF_MEMBER_FUNC) { - SQUserPointer self; - if (SQ_FAILED(sq_getinstanceup(vm, 1, &self, 0))) + ClassInstanceData* classInstanceData; + if (SQ_FAILED(sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0))) { - return sq_throwerror(vm, "Expected class userpointer"); + return SQ_ERROR; } - if (!self) + if (!classInstanceData) { return sq_throwerror(vm, "Accessed null instance"); } - instance = ((ClassInstanceData*)self)->instance; + // check that the type of self, or any basetype, matches the function description + ScriptClassDesc_t *selfType = classInstanceData->desc; + while (selfType != pFunc->m_desc.m_pScriptClassDesc) + { + if (!selfType) + { + return sq_throwerror(vm, "Mismatched instance type"); + } + selfType = selfType->m_pBaseDesc; + Assert(selfType != classInstanceData->desc); // there should be no infinite loop + } + + instance = classInstanceData->instance; } ScriptVariant_t script_retval; From 655679e7da51e8e82af6f906e99b382a8e8eba44 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Mon, 23 Jun 2025 20:49:23 +0200 Subject: [PATCH 03/11] Check type of SQVector construction instance This prevents manual invocations of the Vector.constructor with an invalid value. --- sp/src/vscript/vscript_squirrel.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 39139fec..f2a27755 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -327,7 +327,16 @@ namespace SQVector } SQUserPointer p; - sq_getinstanceup(vm, 1, &p, 0); + if (SQ_FAILED(sq_getinstanceup(vm, 1, &p, 0))) + { + return SQ_ERROR; + } + + if (!p) + { + return sq_throwerror(vm, "Accessed null instance"); + } + new (p) Vector(x, y, z); return 0; From ca7bc5da5718e864992f6be0a5c6e9b0e0dfa281 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Mon, 23 Jun 2025 20:51:23 +0200 Subject: [PATCH 04/11] Guard Squirrel constructor_stub() invocations from invalid class parameters This prevents manual invocations of the native class constructor for non-class values or non-native classes. --- sp/src/vscript/vscript_squirrel.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index f2a27755..8acba22f 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1548,7 +1548,15 @@ SQInteger destructor_stub_instance(SQUserPointer p, SQInteger size) SQInteger constructor_stub(HSQUIRRELVM vm) { ScriptClassDesc_t* pClassDesc = nullptr; - sq_gettypetag(vm, 1, (SQUserPointer*)&pClassDesc); + if (SQ_FAILED(sq_gettypetag(vm, 1, (SQUserPointer*)&pClassDesc))) + { + return sq_throwerror(vm, "Expected native class"); + } + + if (!pClassDesc || (void*)pClassDesc == TYPETAG_VECTOR) + { + return sq_throwerror(vm, "Unable to obtain native class description"); + } if (!pClassDesc->m_pfnConstruct) { From 9c494b6eebfa5c2baec7dae907e5af7b455e0839 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 1 Jul 2025 09:40:57 +0200 Subject: [PATCH 05/11] fxp streaml --- sp/src/vscript/vscript_squirrel.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 8acba22f..6856a2a7 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1332,15 +1332,10 @@ SQInteger function_stub(HSQUIRRELVM vm) { SQInteger top = sq_gettop(vm); - SQUserPointer userptr = nullptr; - if (SQ_FAILED(sq_getuserpointer(vm, top, &userptr))) - { - return sq_throwerror(vm, "Expected userpointer"); - } + ScriptFunctionBinding_t* pFunc = nullptr; + sq_getuserpointer(vm, top, (SQUserPointer*)&pFunc); - Assert(userptr); - - ScriptFunctionBinding_t* pFunc = (ScriptFunctionBinding_t*)userptr; + Assert(pFunc); int nargs = pFunc->m_desc.m_Parameters.Count(); int nLastHScriptIdx = -1; From 9c740a891e4734ff9d9a31d3f1c00086128e6eb2 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Mon, 23 Jun 2025 20:55:13 +0200 Subject: [PATCH 06/11] Check type of Squirrel constructor_stub() instance This prevents manual invocations of the native class constructor with an invalid value. --- sp/src/vscript/vscript_squirrel.cpp | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 6856a2a7..8cbcb35e 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1562,15 +1562,23 @@ SQInteger constructor_stub(HSQUIRRELVM vm) Assert(pSquirrelVM); sq_resetobject(&pSquirrelVM->lastError_); - - void* instance = pClassDesc->m_pfnConstruct(); - - // expect construction to always succeed - Assert(sq_isnull(pSquirrelVM->lastError_)); - { SQUserPointer p; - sq_getinstanceup(vm, 1, &p, 0); + if (SQ_FAILED(sq_getinstanceup(vm, 1, &p, 0))) + { + return SQ_ERROR; + } + + if (!p) + { + return sq_throwerror(vm, "Accessed null instance"); + } + + void* instance = pClassDesc->m_pfnConstruct(); + + // expect construction to always succeed + Assert(sq_isnull(pSquirrelVM->lastError_)); + new(p) ClassInstanceData(instance, pClassDesc, nullptr, true); } From 503fdd2ee372f2dc04ab5957367372e03aa01728 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 24 Jun 2025 20:54:01 +0200 Subject: [PATCH 07/11] Fix SQVector member access check --- sp/src/vscript/vscript_squirrel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 8cbcb35e..0664fea8 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -352,7 +352,7 @@ namespace SQVector return sq_throwerror(vm, "Expected Vector._get(string)"); } - if (key[0] < 'x' || key['0'] > 'z' || key[1] != '\0') + if (key[0] < 'x' || key[0] > 'z' || key[1] != '\0') { return sqstd_throwerrorf(vm, "the index '%.50s' does not exist", key); } @@ -378,7 +378,7 @@ namespace SQVector return sq_throwerror(vm, "Expected Vector._set(string)"); } - if (key[0] < 'x' || key['0'] > 'z' || key[1] != '\0') + if (key[0] < 'x' || key[0] > 'z' || key[1] != '\0') { return sqstd_throwerrorf(vm, "the index '%.50s' does not exist", key); } From be3ad93edba0b579776f47da2ece34004354abad Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 24 Jun 2025 20:54:34 +0200 Subject: [PATCH 08/11] Remove redundant typetag check from vscript_squirrel getVariant() --- sp/src/vscript/vscript_squirrel.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 0664fea8..ee0c0102 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1300,10 +1300,7 @@ bool getVariant(HSQUIRRELVM vm, SQInteger idx, ScriptVariant_t& variant) case OT_INSTANCE: { Vector* v = nullptr; - SQUserPointer tag; - if (SQ_SUCCEEDED(sq_gettypetag(vm, idx, &tag)) && - tag == TYPETAG_VECTOR && - SQ_SUCCEEDED(sq_getinstanceup(vm, idx, (SQUserPointer*)&v, TYPETAG_VECTOR))) + if (SQ_SUCCEEDED(sq_getinstanceup(vm, idx, (SQUserPointer*)&v, TYPETAG_VECTOR))) { variant.Free(); variant = (Vector*)malloc(sizeof(Vector)); From 9c38e4e2951805494ed48205ab914c1660d6e396 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 24 Jun 2025 20:56:26 +0200 Subject: [PATCH 09/11] Remove redundant sq_resetobject() for SquirrelVM::lastError_ The function_stub() resets it when used. --- sp/src/vscript/vscript_squirrel.cpp | 40 +++++++++++++---------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index ee0c0102..c3d7897d 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1460,8 +1460,6 @@ SQInteger function_stub(HSQUIRRELVM vm) SquirrelVM* pSquirrelVM = (SquirrelVM*)sq_getsharedforeignptr(vm); Assert(pSquirrelVM); - sq_resetobject(&pSquirrelVM->lastError_); - bool call_success = (*pFunc->m_pfnBinding)(pFunc->m_pFunction, instance, params.Base(), nargs, pFunc->m_desc.m_ReturnType == FIELD_VOID ? nullptr : &script_retval, script_retval_storage); Assert(call_success); @@ -1555,29 +1553,27 @@ SQInteger constructor_stub(HSQUIRRELVM vm) return sqstd_throwerrorf(vm, "Unable to construct instances of %s", pClassDesc->m_pszScriptName); } + SQUserPointer p; + if (SQ_FAILED(sq_getinstanceup(vm, 1, &p, 0))) + { + return SQ_ERROR; + } + + if (!p) + { + return sq_throwerror(vm, "Accessed null instance"); + } + + void* instance = pClassDesc->m_pfnConstruct(); + +#ifdef DBGFLAG_ASSERT SquirrelVM* pSquirrelVM = (SquirrelVM*)sq_getsharedforeignptr(vm); Assert(pSquirrelVM); + // expect construction to always succeed + Assert(sq_isnull(pSquirrelVM->lastError_)); +#endif - sq_resetobject(&pSquirrelVM->lastError_); - { - SQUserPointer p; - if (SQ_FAILED(sq_getinstanceup(vm, 1, &p, 0))) - { - return SQ_ERROR; - } - - if (!p) - { - return sq_throwerror(vm, "Accessed null instance"); - } - - void* instance = pClassDesc->m_pfnConstruct(); - - // expect construction to always succeed - Assert(sq_isnull(pSquirrelVM->lastError_)); - - new(p) ClassInstanceData(instance, pClassDesc, nullptr, true); - } + new(p) ClassInstanceData(instance, pClassDesc, nullptr, true); sq_setreleasehook(vm, 1, &destructor_stub); From 3301edc54d95b10373bb5a4a4e97124b3c753937 Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 24 Jun 2025 20:58:54 +0200 Subject: [PATCH 10/11] Guard get/set/tostring_stub() against illegal Squirrel instances Also streamline SQUserPointer usage in sq_getinstanceup() and sq_getuserpointer() calls to write directly to a pointer of the expected type. --- sp/src/vscript/vscript_squirrel.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index c3d7897d..7c2861a6 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1583,7 +1583,10 @@ SQInteger constructor_stub(HSQUIRRELVM vm) SQInteger tostring_stub(HSQUIRRELVM vm) { ClassInstanceData* classInstanceData = nullptr; - sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0); + if (SQ_FAILED(sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0))) + { + return SQ_ERROR; + } char buffer[128] = ""; @@ -1613,7 +1616,10 @@ SQInteger tostring_stub(HSQUIRRELVM vm) SQInteger get_stub(HSQUIRRELVM vm) { ClassInstanceData* classInstanceData = nullptr; - sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0); + if (SQ_FAILED(sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0))) + { + return SQ_ERROR; + } const char* key = nullptr; sq_getstring(vm, 2, &key); @@ -1645,7 +1651,10 @@ SQInteger get_stub(HSQUIRRELVM vm) SQInteger set_stub(HSQUIRRELVM vm) { ClassInstanceData* classInstanceData = nullptr; - sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0); + if (SQ_FAILED(sq_getinstanceup(vm, 1, (SQUserPointer*)&classInstanceData, 0))) + { + return SQ_ERROR; + } const char* key = nullptr; sq_getstring(vm, 2, &key); @@ -2741,10 +2750,8 @@ void SquirrelVM::SetInstanceUniqeId(HSCRIPT hInstance, const char* pszId) HSQOBJECT* obj = (HSQOBJECT*)hInstance; sq_pushobject(vm_, *obj); - SQUserPointer self; - sq_getinstanceup(vm_, -1, &self, nullptr); - - auto classInstanceData = (ClassInstanceData*)self; + ClassInstanceData* classInstanceData; + sq_getinstanceup(vm_, -1, (SQUserPointer*)&classInstanceData, nullptr); classInstanceData->instanceId = pszId; @@ -2802,11 +2809,10 @@ void* SquirrelVM::GetInstanceValue(HSCRIPT hInstance, ScriptClassDesc_t* pExpect } sq_pushobject(vm_, *obj); - SQUserPointer self; - sq_getinstanceup(vm_, -1, &self, nullptr); + ClassInstanceData* classInstanceData; + sq_getinstanceup(vm_, -1, (SQUserPointer*)&classInstanceData, nullptr); sq_pop(vm_, 1); - auto classInstanceData = (ClassInstanceData*)self; if (!classInstanceData) { From c659af5944c0dacbd4b7d46d1d91defb81b6c19f Mon Sep 17 00:00:00 2001 From: Alexander 'z33ky' Hirsch <1zeeky@gmail.com> Date: Tue, 24 Jun 2025 21:07:57 +0200 Subject: [PATCH 11/11] Release SquirrelVM::lastError_ after pushing it to the VM stack This fixes a memory leak. --- sp/src/vscript/vscript_squirrel.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 7c2861a6..b689f757 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -1469,6 +1469,7 @@ SQInteger function_stub(HSQUIRRELVM vm) if (!sq_isnull(pSquirrelVM->lastError_)) { sq_pushobject(vm, pSquirrelVM->lastError_); + sq_release(vm, &pSquirrelVM->lastError_); sq_resetobject(&pSquirrelVM->lastError_); sq_retval = sq_throwobject(vm); }