0

I have a NDIS 6 filter driver. And I need to compare two NDIS_STRING in a case-insensitive manner at IRQL = DISPATCH_LEVEL. I know the RtlEqualUnicodeString function can compare strings in a case-insensitive manner. But it's only callable at PASSIVE_LEVEL.

So I have to write my own function using the basic memory copy way. And I found my function is not well functioned, because some of my users complain that this function returns FALSE when it's supposed to return TRUE. So there should be some bugs in my code. But I didn't find it by myself.

BOOLEAN
NPF_EqualAdapterName(
    PNDIS_STRING s1,
    PNDIS_STRING s2
    )
{
    int i;
    BOOLEAN bResult;
    TRACE_ENTER();

    if (s1->Length != s2->Length)
    {
        IF_LOUD(DbgPrint("NPF_EqualAdapterName: length not the same, s1->Length = %d, s2->Length = %d\n", s1->Length, s2->Length);)
        TRACE_EXIT();
        return FALSE;
    }

    for (i = 0; i < s2->Length / 2; i ++)
    {
        if (s1->Buffer[i] >= L'A' && s1->Buffer[i] <= L'Z')
        {
            s1->Buffer[i] += (L'a' - L'A');
        }
        if (s2->Buffer[i] >= L'A' && s2->Buffer[i] <= L'Z')
        {
            s2->Buffer[i] += (L'a' - L'A');
        }
    }

    bResult = RtlEqualMemory(s1->Buffer, s2->Buffer, s2->Length);
    IF_LOUD(DbgPrint("NPF_EqualAdapterName: bResult = %d, s1 = %ws, s2 = %ws\n", i, bResult, s1->Buffer, s2->Buffer);)
    TRACE_EXIT();
    return bResult;
}

The entire code is here: https://github.com/nmap/npcap/blob/master/packetWin7/npf/npf/Openclos.c, if you want to know it.

So my question is simply, is there any bugs in the above code? Thanks!

UPDATE:

The adapter names (e.g., s1 and s2) are some GUIDs like {1CC605D7-B674-440B-9D58-3F68E0529B54}. They can be upper-case or lower-case. So they are definitely English.

An idea of using an index or key would be storing the names in GUID structure instead of strings. I noticed that Windows has provided RtlStringFromGUID and RtlGUIDFromString functions. However, these two functions also only work at IRQL = PASSIVE_LEVEL.

And much of my code just runs under DISPATCH_LEVEL. So I'm afraid storing in GUID is not a good idea.

hsluoyz
  • 2,739
  • 5
  • 35
  • 59
  • My main concern would be that it modifies the input strings. You'd need to be very careful to make sure that you're allowed to do that. As for debugging it, you should start by finding out which strings are failing to compare as expected. (Perhaps they are non-English letters?) – Harry Johnston Mar 25 '16 at 06:23
  • See my update. I'm able to change the chars in the string. If not allowed, I should be seeing a BSoD. I'm already asking my user to provide the debug traces to see what ``s1`` and ``s2`` really are. – hsluoyz Mar 25 '16 at 12:40
  • Changing content you're not supposed to won't necessarily cause a bluescreen, not unless the content is actually stored in read-only memory. It might instead cause the OS to malfunction in more subtle ways. But that doesn't seem likely to be the cause of your problem. – Harry Johnston Mar 25 '16 at 22:54
  • More importantly: what strings are failing? You may need to write some code that uses DbgPrint to report the content of the two strings in hexadecimal, both before and after you lower-case them. (Also note that the use of `%ws` is only allowed at PASSIVE_LEVEL, so you should change that.) – Harry Johnston Mar 25 '16 at 22:58
  • Then how should I ``DbgPrint`` a ``PNDIS_STRING`` without ``%ws``? – hsluoyz Mar 26 '16 at 01:16
  • I can make sure the content of ``s1`` and ``s2`` are both allocated by my driver on a ``NonPagedPool``, is this better? – hsluoyz Mar 26 '16 at 01:17
  • Presumably it's the pages containing the code for handling `%ws` that are the problem, same as when you were calling RtlEqualUnicodeString. Just write a routine that converts the strings to hexadecimal, there's [some code here](http://stackoverflow.com/a/31367139/886887) that should work, you'll just need to replace malloc() with a kernel-mode memory allocation function. – Harry Johnston Mar 27 '16 at 00:50

0 Answers0