Skip to content
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

Comparing unrelated pointers is undefined behavior #919

Open
imaami opened this issue Dec 25, 2024 · 1 comment
Open

Comparing unrelated pointers is undefined behavior #919

imaami opened this issue Dec 25, 2024 · 1 comment

Comments

@imaami
Copy link

imaami commented Dec 25, 2024

This is incorrect C:

cJSON/cJSON.c

Line 425 in 12c4bf1

if (!( valuestring + v1_len < object->valuestring || object->valuestring + v2_len < valuestring ))

The C17 standard (6.5.8.5) says this about pointer comparisons:

When two pointers are compared, the result depends on the relative locations in the address space of the objects pointed to. If two pointers to object types both point to the same object, or both point one past the last element of the same array object, they compare equal. If the objects pointed to are members of the same aggregate object, pointers to structure members declared later compare greater than pointers to members declared earlier in the structure, and pointers to array elements with larger subscript values compare greater than pointers to elements of the same array with lower subscript values. All pointers to members of the same union object compare equal. If the expression P points to an element of an array object and the expression Q points to the last element of the same array object, the pointer expression Q+1 compares greater than P. In all other cases, the behavior is undefined.

In other words you can't legally determine whether or not two unrelated arrays overlap. It is only well-defined if those arrays belong to the same object. The current test case is free of UB because it happens to use pointers to addresses within the same array. That's not representative of real use.

@imaami
Copy link
Author

imaami commented Dec 25, 2024

The fix is to just not bother with the janky comparison logic and use memmove() instead of strcpy(). Overlapping strings are not a problem after that. You already take the strlen() of the strings, memmove() is no less suitable than strcpy() when you've already gone and found the null terminators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant