bpo-41524: fix pointer bug in PyOS_mystr{n}icmp (GH-21845)

wmeehan · blurb-it[bot] · web-flow · commit 97eaf2b5e5c8 · 2020-08-27T14:45:25.000+09:00
* bpo-41524: fix pointer bug in PyOS_mystr{n}icmp The existing implementations of PyOS_mystrnicmp and PyOS_mystricmp can increment pointers beyond the end of a string. This commit fixes those cases by moving the mutation out of the condition. * 📜🤖 Added by blurb_it. * Address comments Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
diff --git a/Misc/NEWS.d/next/C API/2020-08-12-17-09-06.bpo-41524.u6Xfr2.rst b/Misc/NEWS.d/next/C API/2020-08-12-17-09-06.bpo-41524.u6Xfr2.rst
@@ -0,0 +1,2 @@
+Fix bug in PyOS_mystrnicmp and PyOS_mystricmp that incremented
+pointers beyond the end of a string.
diff --git a/Python/pystrcmp.c b/Python/pystrcmp.c
@@ -6,21 +6,25 @@
 int
 PyOS_mystrnicmp(const char *s1, const char *s2, Py_ssize_t size)
 {
+    const unsigned char *p1, *p2;
     if (size == 0)
         return 0;
-    while ((--size > 0) &&
-           (tolower((unsigned)*s1) == tolower((unsigned)*s2))) {
-        if (!*s1++ || !*s2++)
-            break;
+    p1 = (const unsigned char *)s1;
+    p2 = (const unsigned char *)s2;
+    for (; (--size > 0) && *p1 && *p2 && (tolower(*p1) == tolower(*p2));
+         p1++, p2++) {
+        ;
     }
-    return tolower((unsigned)*s1) - tolower((unsigned)*s2);
+    return tolower(*p1) - tolower(*p2);
 }
 
 int
 PyOS_mystricmp(const char *s1, const char *s2)
 {
-    while (*s1 && (tolower((unsigned)*s1++) == tolower((unsigned)*s2++))) {
+    const unsigned char *p1 = (const unsigned char *)s1;
+    const unsigned char *p2 = (const unsigned char *)s2;
+    for (; *p1 && *p2 && (tolower(*p1) == tolower(*p2)); p1++, p2++) {
         ;
     }
-    return (tolower((unsigned)*s1) - tolower((unsigned)*s2));
+    return (tolower(*p1) - tolower(*p2));
 }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Fix bug in PyOS_mystrnicmp and PyOS_mystricmp that incremented`
	`2`	`+pointers beyond the end of a string.`
Original file line number	Diff line number	Diff line change
`@@ -6,21 +6,25 @@`
`6`	`6`	`int`
`7`	`7`	`PyOS_mystrnicmp(const char s1, const char s2, Py_ssize_t size)`
`8`	`8`	`{`
	`9`	`+ const unsigned char p1, p2;`
`9`	`10`	`if (size == 0)`
`10`	`11`	`return 0;`
`11`		`- while ((--size > 0) &&`
`12`		`- (tolower((unsigned)s1) == tolower((unsigned)s2))) {`
`13`		`- if (!s1++ \|\| !s2++)`
`14`		`- break;`
	`12`	`+ p1 = (const unsigned char *)s1;`
	`13`	`+ p2 = (const unsigned char *)s2;`
	`14`	`+ for (; (--size > 0) && p1 && p2 && (tolower(p1) == tolower(p2));`
	`15`	`+ p1++, p2++) {`
	`16`	`+ ;`
`15`	`17`	`}`
`16`		`- return tolower((unsigned)s1) - tolower((unsigned)s2);`
	`18`	`+ return tolower(p1) - tolower(p2);`
`17`	`19`	`}`
`18`	`20`
`19`	`21`	`int`
`20`	`22`	`PyOS_mystricmp(const char s1, const char s2)`
`21`	`23`	`{`
`22`		`- while (s1 && (tolower((unsigned)s1++) == tolower((unsigned)*s2++))) {`
	`24`	`+ const unsigned char p1 = (const unsigned char )s1;`
	`25`	`+ const unsigned char p2 = (const unsigned char )s2;`
	`26`	`+ for (; p1 && p2 && (tolower(p1) == tolower(p2)); p1++, p2++) {`
`23`	`27`	`;`
`24`	`28`	`}`
`25`		`- return (tolower((unsigned)s1) - tolower((unsigned)s2));`
	`29`	`+ return (tolower(p1) - tolower(p2));`
`26`	`30`	`}`