The comment before fixupMultiSZ and countString states:

** Note that fixupMultiSZ and countString have both had changes
** made to support "incorrect strings". The registry specification
** calls for strings to be terminated with 2 null bytes. It seems
** some commercial packages install strings which don't conform,
** causing this code to fail - however, "regedit" etc still work
** with these strings (ie only we don't!).

As indicated in the comments, the two functions dutifully check the supplied length parameter and do not trust the data to be in the correct format.

... except for the inner loop in fixupMultiSZ, which reads:

                for(; *P != '\0'; P++)
                        ;

It should be the same as the inner loop of countStrings:

            for (; P < Q && *P != '\0'; P++)
                    ;

@tim is this something that you can comment on?

@Steve/Zach FYI.

I've created a PR for this issue.

There's still a potential problem when Reg2Py calls wcslen(str[index]). This could be addressed by having fixupMultiSZ take an int array to store the length of each string. For example:

    static void
    fixupMultiSZ(wchar_t **strings, int *lengths, wchar_t *data, int len)
    {
        wchar_t *P, *Q = data + len;
        int i;

        for (P = data, i = 0; P < Q && *P; P++, i++) {
            strings[i] = P;
            lengths[i] = 0;
            for (; P < Q && *P; P++) {
                lengths[i]++;
            }
        }
    }

We'd have to allocate the lengths array in Reg2Py, like we do for the strings array. Also, we can remove the overflow error check prior to PyUnicode_FromWideChar. The longest possible length is retDataSize / 2, which occurs if a single string is stored without any null terminators.

New changeset 56ed864 by Steve Dower (Zackery Spytz) in branch 'master':
bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687)
56ed864

Thanks Zackery! I've merged this main part of the fix (though it requires a manual backport to 2.7). As it's a buffer overrun, I've sent it back to 3.6 as well.

Eryk - thanks for the additional detail. I wonder whether it would be just as easy to guarantee an over-allocation in this case and force a null terminator? (In fact, that would probably have handled the same case that Zackery just fixed, but we didn't have a patch ready for that approach)

New changeset 7038dee by Miss Islington (bot) in branch '3.7':
bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687)
7038dee

New changeset 84efbae by Miss Islington (bot) (Zackery Spytz) in branch '2.7':
[2.7] bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687) (GH-12916)
84efbae

Declaring this done - Ned can take the backport to 3.6 if/when he feels like it.

New changeset dadc347 by Ned Deily (Miss Islington (bot)) in branch '3.6':
bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687) (GH-12910)
dadc347