[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Ned Deily]

Change by Ned Deily :


--
versions: +Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Ned Deily]

Ned Deily  added the comment:


New changeset dadc3478950c389c120fb16f44e5a29cc43f 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)
https://github.com/python/cpython/commit/dadc3478950c389c120fb16f44e5a29cc43f


--
nosy: +ned.deily

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Steve Dower]

Steve Dower  added the comment:

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

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[miss-islington]

miss-islington  added the comment:


New changeset 84efbaecaf50b771cc7a95fd9dd9602bd31de305 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)
https://github.com/python/cpython/commit/84efbaecaf50b771cc7a95fd9dd9602bd31de305


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Zackery Spytz]

Change by Zackery Spytz :


--
pull_requests: +12842
stage: backport needed -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[miss-islington]

miss-islington  added the comment:


New changeset 7038deed09784a03e2a7bad500f0054d29876ae7 by Miss Islington (bot) 
in branch '3.7':
bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687)
https://github.com/python/cpython/commit/7038deed09784a03e2a7bad500f0054d29876ae7


--
nosy: +miss-islington

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Steve Dower]

Steve Dower  added the comment:

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)

--
stage: patch review -> backport needed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[miss-islington]

Change by miss-islington :


--
pull_requests: +12837

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[miss-islington]

Change by miss-islington :


--
pull_requests: +12836

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Steve Dower]

Steve Dower  added the comment:


New changeset 56ed86490cb8221c874d432461d77702437f63e5 by Steve Dower (Zackery 
Spytz) in branch 'master':
bpo-9194: Fix the bounds checking in winreg.c's fixupMultiSZ() (GH-12687)
https://github.com/python/cpython/commit/56ed86490cb8221c874d432461d77702437f63e5


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Eryk Sun]

Eryk Sun  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Zackery Spytz]

Zackery Spytz  added the comment:

I've created a PR for this issue.

--
nosy: +ZackerySpytz, eryksun
versions: +Python 3.7, Python 3.8 -Python 3.2, Python 3.3, Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Zackery Spytz]

Change by Zackery Spytz :


--
keywords: +patch
pull_requests: +12614
stage: test needed -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P < Q in the inner loop

[Mark Lawrence]

Change by Mark Lawrence :


--
nosy:  -BreamoreBoy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Mark Lawrence]

Mark Lawrence added the comment:

@Steve/Zach FYI.

--
nosy: +steve.dower, zach.ware

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Mark Lawrence]

Mark Lawrence added the comment:

@Tim is this something that you can comment on?

--
nosy: +BreamoreBoy, tim.golden

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Brian Curtin]

Changes by Brian Curtin br...@python.org:


--
nosy:  -brian.curtin

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
versions: +Python 3.3, Python 3.4 -Python 2.6, Python 3.1

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Daniel Stutzbach]

New submission from Daniel Stutzbach dan...@stutzbachenterprises.com:

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++)
;

--
messages: 109511
nosy: stutzbach
priority: normal
severity: normal
status: open
title: winreg:fixupMultiSZ should check that P  Q in the inner loop
versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue9194] winreg:fixupMultiSZ should check that P Q in the inner loop

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
components: +Extension Modules, Windows
nosy: +brian.curtin
stage:  - unit test needed
type:  - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue9194
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com