[Bug] Control Flow Hijacking Issue in sys_device_close System Call in RT-Thread

RT-Thread Version

5.1.0

Hardware Type/Architectures

N/A

Develop Toolchain

Other

Describe the bug

Summary

I have identified a critical vulnerability in the sys_device_close system call in RT-Thread. I am opening this issue for your review, as I could not find a reporting email in the security policy of this repository. This vulnerability stems from insufficient pointer validation and could allow an attacker to achieve arbitrary code execution through control flow hijacking. The issue is particularly severe as it could be exploited by a compromised user thread to gain elevated privileges.

Vulnerable Code Location

The vulnerability is present in the following files:

1. components/lwp/lwp_syscall.c: sys_device_close function
2. components/drivers/core/device.c: rt_device_close function and device_close macro

The call graph of this vulnerability is as follows:

// Macro definition in device.c
#define device_close    (dev->close)

// System call in lwp_syscall.c
sysret_t sys_device_close(rt_device_t dev)
{
    return rt_device_close(dev);
}

// Implementation in device.c
rt_err_t rt_device_close(rt_device_t dev)
{
    rt_err_t result = RT_EOK;
    RT_ASSERT(dev != RT_NULL);
    RT_ASSERT(rt_object_get_type(&dev->parent) == RT_Object_Class_Device);

    if (dev->ref_count == 0)
        return -RT_ERROR;

    dev->ref_count--;

    if (dev->ref_count != 0)
        return RT_EOK;

    if (device_close != RT_NULL)  // This expands to: if (dev->close != RT_NULL)
    {
        result = device_close(dev);  // This expands to: result = (dev->close)(dev);
    }
    // ...
}

Vulnerability Description

1. The vulnerability exists in the rt_device_close function's handling of the device_close function pointer. The code only performs a NULL check on the function pointer but fails to verify whether the pointer points to valid memory or a valid function. This oversight can lead to control flow hijacking when the function pointer is called. The issue is particularly critical because:

   1. The device_close is a macro that expands to dev->close, making it a function pointer stored in the device structure
   2. The code only checks if the pointer is NULL (if (device_close != RT_NULL))
   3. No validation is performed to ensure the pointer points to valid memory or a valid function before calling it
   4. When calling device_close(dev), which expands to (dev->close)(dev), control flow can be hijacked if the pointer has been corrupted

Impact

This vulnerability has severe security implications:

1. Arbitrary Code Execution: An attacker could potentially corrupt the dev->close function pointer to point to arbitrary memory locations, leading to arbitrary code execution.

2. Privilege Escalation: Since this vulnerability exists in the kernel space, successful exploitation could lead to privilege escalation, allowing an attacker to execute code with kernel privileges.

3. System Compromise: The ability to execute arbitrary code in kernel space could lead to complete system compromise, including:

   • Bypassing security mechanisms
   • Accessing sensitive data
   • Installing persistent malware
   • Disabling security features

Mitigation

1. Implement proper validation of function pointers before calling them
2. Add additional checks to verify the integrity of the device structure
3. Consider implementing Control Flow Integrity (CFI) mechanisms
4. Add bounds checking for pointer dereferencing

References

1. RT-Thread source code: components/lwp/lwp_syscall.c
2. RT-Thread source code: components/drivers/core/device.c

Other additional context

No response

Start by reviewing sys_device_close in components/lwp/lwp_syscall.c and rt_device_close plus the device_close macro in components/drivers/core/device.c. Trace how device pointers and function pointers are created and validated, then determine whether the reported scenario is reproducible on a supported configuration. Done requires an agreed security mitigation and evidence that the affected call path is covered.