Skip to content

Fix ownership issue for string_pointer and buffer overrun issue for string_array. (#472) #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fixed set_string_pointer to always create a new string object.
  • Loading branch information
CookStar committed May 21, 2023
commit 029391b87b8ed135e826a414b01a2d73e99dea1d
24 changes: 21 additions & 3 deletions addons/source-python/packages/source-python/memory/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,16 @@ def fset(ptr, value):

# Handle native type
else:
getattr(ptr, 'set_' + type_name)(value, offset)
# Handle string pointer type
if type_name == Type.STRING_POINTER:
string_pointer = ptr.set_string_pointer(value, offset)
string_pointer.auto_dealloc = True

# Make sure the value will not deallocate as long as it is
# part of this object
ptr._pointer_values[offset] = string_pointer
else:
getattr(ptr, 'set_' + type_name)(value, offset)

return property(fget, fset, None, doc)

Expand Down Expand Up @@ -521,8 +530,17 @@ def fset(ptr, value):
# Set the pointer
ptr.set_pointer(instance_ptr, offset)

# Set the value
getattr(instance_ptr, 'set_' + type_name)(value)
# Handle string pointer type
if type_name == Type.STRING_POINTER:
string_pointer = instance_ptr.set_string_pointer(value, offset)
string_pointer.auto_dealloc = True

# Make sure the value will not deallocate as long as it is
# part of this object
ptr._pointer_values[offset] = string_pointer
else:
# Set the value
getattr(instance_ptr, 'set_' + type_name)(value)

return property(fget, fset, None, doc)

Expand Down
23 changes: 23 additions & 0 deletions src/core/modules/memory/memory_pointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,29 @@ CPointer::CPointer(unsigned long ulAddr /* = 0 */, bool bAutoDealloc /* false */
m_bAutoDealloc = bAutoDealloc;
}

const char * CPointer::GetStringPointer(int iOffset /* = 0 */)
{
Validate();
const char * result;
TRY_SEGV()
result = *(const char **) (m_ulAddr + iOffset);
EXCEPT_SEGV()
return result;
}

CPointer * CPointer::SetStringPointer(str oString, int iOffset /* = 0 */)
{
Validate();
unsigned long length = len(oString) + 1;
CPointer * pPtr = new CPointer((unsigned long) UTIL_Alloc(length), false);
char * value = (char *) pPtr->m_ulAddr;
memcpy(value, extract<const char *>(oString), length);
TRY_SEGV()
*(const char **) (m_ulAddr + iOffset) = value;
EXCEPT_SEGV()
return pPtr;
}

const char * CPointer::GetStringArray(int iOffset /* = 0 */)
{
Validate();
Expand Down
3 changes: 3 additions & 0 deletions src/core/modules/memory/memory_pointer.h
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ class CPointer
EXCEPT_SEGV()
}

const char * GetStringPointer(int iOffset = 0);
CPointer* SetStringPointer(str oString, int iOffset = 0);

const char * GetStringArray(int iOffset = 0);
void SetStringArray(char* szText, int iOffset = 0);

Expand Down
17 changes: 15 additions & 2 deletions src/core/modules/memory/memory_wrap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ void export_pointer(scope _memory)
EXPOSE_GET_SET_TYPE(ulong_long, unsigned long long)
EXPOSE_GET_SET_TYPE(float, float)
EXPOSE_GET_SET_TYPE(double, double)
EXPOSE_GET_SET_TYPE(string_pointer, const char*)

.def("get_pointer",
&CPointer::GetPtr,
Expand All @@ -210,6 +209,12 @@ void export_pointer(scope _memory)
manage_new_object_policy()
)

.def("get_string_pointer",
&CPointer::GetStringPointer,
"Returns the value at the memory location.",
(arg("offset")=0)
)

.def("get_string_array",
&CPointer::GetStringArray,
"Returns the value at the memory location.",
Expand All @@ -222,10 +227,17 @@ void export_pointer(scope _memory)
("value", arg("offset")=0)
)

.def("set_string_pointer",
&CPointer::SetStringPointer,
"Sets the value at the given memory location. This string object must be deallocated by the user. Returns the string object.",
("value", arg( "offset")=0),
manage_new_object_policy()
)

.def("set_string_array",
&CPointer::SetStringArray,
"Sets the value at the given memory location.",
("value",arg( "offset")=0)
("value", arg( "offset")=0)
)

// Other methods
Expand Down Expand Up @@ -1048,6 +1060,7 @@ void export_global_variables(scope _memory)
ADD_NATIVE_TYPE_SIZE("DOUBLE", double)
ADD_NATIVE_TYPE_SIZE("POINTER", void*)
ADD_NATIVE_TYPE_SIZE("STRING", char*)
ADD_NATIVE_TYPE_SIZE("STRING_POINTER", char*)

_memory.attr("NULL") = object(CPointer());

Expand Down