Fixed memory safety of prop hider

This commit is contained in:
snake-4
2024-05-03 19:54:40 +02:00
parent 8c7f376f16
commit 02aae1bbd7
3 changed files with 65 additions and 37 deletions

View File

@@ -9,21 +9,29 @@
ret (*old_##func)(__VA_ARGS__) = nullptr; \
ret new_##func(__VA_ARGS__)
#define ASSERT_LOG(tag, expr) \
if (!(expr)) \
{ \
#define ASSERT_LOG(tag, expr) \
if (!(expr)) \
{ \
LOGE("%s:%d Assertion %s failed. %d:%s", #tag, __LINE__, #expr, errno, std::strerror(errno)); \
}
#define ASSERT_DO(tag, expr, ret) \
if (!(expr)) \
{ \
#define ASSERT_DO(tag, expr, ret) \
if (!(expr)) \
{ \
LOGE("%s:%d Assertion %s failed. %d:%s", #tag, __LINE__, #expr, errno, std::strerror(errno)); \
ret; \
ret; \
}
namespace Utils
{
/*
* Always null terminates dest if dest_size is at least 1.
* Writes at most dest_size bytes to dest including null terminator.
* Reads at most dest_size bytes from src.
* Returns strlen(dest)
*/
size_t safeStringCopy(char *dest, const char *src, size_t dest_size);
bool switchMountNS(int pid);
int isUserAppUID(int uid);
bool hookPLTByName(zygisk::Api *api, const std::string &libName, const std::string &symbolName, void *hookFunc, void **origFunc);

View File

@@ -1,4 +1,5 @@
#include <string>
#include <cstring>
#include <string_view>
#include <set>
#include <atomic>
@@ -168,49 +169,53 @@ void doHideZygisk()
}
}
static bool shouldResetProperty(const prop_info *pi)
{
// Read-write properties or properties with long values should not be reset
if (strncmp(pi->name, "ro.", 3) != 0 || pi->is_long())
return false;
// Check if the serial indicates that it was modified
auto serial = std::atomic_load_explicit(&pi->serial, std::memory_order_relaxed);
if ((serial & 0xFFFFFF) != 0)
return true;
// Check if any characters exist beyond the null-terminated string
size_t length = strnlen(pi->value, PROP_VALUE_MAX);
for (size_t i = length; i < PROP_VALUE_MAX; i++)
{
if (pi->value[i] != '\0')
return true;
}
return false;
}
void doMrProp()
{
static bool isInitialized = false;
static int resetCount = 0;
if (!isInitialized)
{
isInitialized = __system_properties_init() == 0;
}
if (!isInitialized)
{
LOGE("Could not initialize system_properties!");
return;
if (__system_properties_init() == -1)
{
LOGE("Could not initialize system_properties!");
return;
}
isInitialized = true;
}
int ret = __system_property_foreach(
[](const prop_info *pi, void *)
{
if (std::string_view(pi->name).starts_with("ro.") && !pi->is_long())
if (shouldResetProperty(pi))
{
auto serial = std::atomic_load_explicit(&pi->serial, std::memory_order_relaxed);
// Overlapping pointers in strncpy is undefined behavior so make a copy.
char buffer[PROP_VALUE_MAX];
size_t length = Utils::safeStringCopy(buffer, pi->value, PROP_VALUE_MAX);
// Well this is a bit dangerous
bool shouldReset = (serial & 0xFF) != 0;
auto length = strlen(pi->value);
if (!shouldReset)
{
for (size_t i = length; i < PROP_VALUE_MAX; i++)
{
if (pi->value[i] != 0)
{
shouldReset = true;
break;
}
}
}
if (shouldReset)
{
resetCount++;
__system_property_update(const_cast<prop_info *>(pi), pi->value, length);
}
__system_property_update(const_cast<prop_info *>(pi), buffer, length);
resetCount++;
}
},
nullptr);

View File

@@ -1,3 +1,4 @@
#include <string.h>
#include <string>
#include <functional>
#include <format>
@@ -14,6 +15,20 @@
using namespace Utils;
size_t Utils::safeStringCopy(char *dest, const char *src, size_t dest_size)
{
if (dest_size < 1)
return 0;
*dest = 0;
// strncpy does not null terminate, strlcpy fails on non-terminated src
// strncat is relatively safe but may write count+1 because of the null terminator
strncat(dest, src, dest_size - 1);
// strlen is safe here because dest is now null-terminated
return strlen(dest);
}
bool Utils::hookPLTByName(zygisk::Api *api, const std::string &libName, const std::string &symbolName, void *hookFunc, void **origFunc)
{
for (const auto &map : Parsers::parseSelfMaps())