[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

Sebastian Lackner <sebastian at fds-team.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|String compare functions    |CompareStringW should abort
                   |with only one length        |on the first nonmatching
                   |argument can lead to page   |character to avoid invalid
                   |faults.                     |memory access.
           Severity|normal                      |minor

--- Comment #3 from Sebastian Lackner <sebastian at fds-team.de> ---
Hi Dmitry,

sorry for the delayed reply. The main issue with Flash is already fixed by some
of the patches I sent last week. In this case it was possible to avoid the
invalid memory access by changing the way how CompareStringW is called from
Wine code.

Nevertheless I would like to keep this bug report open for the more general
issue, which can also be reproduced using the attached test. Changing priority
to 'minor' because I am not aware of any real world application affected by
this.

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.

* 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.

* 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.


Regards,
Sebastian

-- 
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.