[Bug 37556] CompareStringW should abort on the first nonmatching character to avoid invalid memory access.

[wine-bugs at winehq.org]

https://bugs.winehq.org/show_bug.cgi?id=37556

Dmitry Timoshkov <dmitry at baikal.ru> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #50084|0                           |1
        is obsolete|                            |

--- Comment #4 from Dmitry Timoshkov <dmitry at baikal.ru> ---
Created attachment 50119
  --> https://bugs.winehq.org/attachment.cgi?id=50119
patch

(In reply to Sebastian Lackner from comment #3)

Thanks for the comments Sebastian.

> The patch fixes the issue, but has some problems:
> 
> * First of all, *str1 and *str2 should only be accessed when len1/len2 is >
> 0, so I would suggest to change the order here.

Good catch, attached a fixed version of the patch.

> * The old code in real_length(...) used a loop, the new code uses just an if
> statement. Not sure if there are already tests for that, if not it probably
> makes sense to check what exactly is right. This would make a difference for
> strings with multiple null termination characters.

There are already some tests that fail without real_length(), that's why
I added this call in the first place long time ago.

> * The following code could also behaves a bit different when the strings
> still contain null termination:
> 
> --- snip ---
> ...
>         if (!(flags & SORT_STRINGSORT))
>         {
>             if (*str1 == '-' || *str1 == '\'')
>             {
>                 if (*str2 != '-' && *str2 != '\'')
>                 {
>                     str1++;
>                     len1--;
>                     continue;
>                 }
> ...
> --- snip ---
> 
> At the moment '-' and '\'' at the end of the string are not skipped.

This would need some additional tests. Still there are some tests already
which check strings with embedded '\0'.

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.