`ly_set_rm_index_ordered` crashes when index is out of bounds

[X-ANOY]

Hi, I found a crash in the ly_set_rm_index_ordered function. It seems the function lacks a proper bounds check to ensure that the index being removed is actually within the current range of the set.

Specifically, the LY_CHECK_ARG_RET macro at the start only checks if the set exists and if its count is non-zero, but it doesn't verify index < set->count. When an out-of-bounds index is passed, the code decrements set->count and then reaches the memmove call. Since count is now smaller than index, the subtraction (set->count - index) underflows, resulting in a massive unsigned value for the memmove size.

When running with AddressSanitizer, it reports a huge memory read (around 32GB in my case), which leads to a segfault. Here is a minimal PoC that adds one item to a set and then incorrectly tries to remove index 1:

#include <libyang.h>
#include <stdio.h>
#include <stdlib.h>

int main() {
    struct ly_set *set = NULL;
    LY_ERR ret;

    printf("[*] Initializing libyang set...\n");
    ret = ly_set_new(&set);
    if (ret != LY_SUCCESS) {
        return 1;
    }

    int dummy_data = 123;
    ly_set_add(set, &dummy_data, 0, NULL);
    
    // Triggering the underflow: index 1 on a set with count 1
    ly_set_rm_index_ordered(set, 1, NULL);

    ly_set_free(set, NULL);
    return 0;
}

The ASAN output shows the crash happening exactly during that memmove inside set.c:

==297924==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x506000000060
READ of size 34359738360 at 0x506000000060 thread T0
    #0 0x620a810a3fef in memmove
    #1 0x620a810e534e in ly_set_rm_index_ordered /home/.../Libraries/libyang/src/set.c:244:9
    #2 0x620a810e3c8c in main poc.c:18:5

Updating the initial check to include index < set->count should fix this and prevent the underflow. Thanks for taking a look!

[X-ANOY]

I noticed that ly_set_rm_index has a proper bounds check LY_CHECK_ERR_RET(index >= set->count, LOGARG(NULL, index), LY_EINVAL);, while ly_set_rm_index_ordered is missing it.

[michalvasko]

Right, fixed.