Skip to content

Commit 3a939ff

Browse files
serhiy-storchakaserhiy-storchaka
committed
Fix use-after-free in the unicode-escape decoder with error handler
If the error handler is used, a new bytes object is created to set as the object attribute of UnicodeDecodeError, and that bytes object then replaces the original data. A pointer to the decoded data will became invalid after destroying that temporary bytes object. So we need other way to return the first invalid escape from _PyUnicode_DecodeUnicodeEscapeInternal(). _PyBytes_DecodeEscape() does not have such issue, because it does not use the error handlers registry, but it should be changed for compatibility with _PyUnicode_DecodeUnicodeEscapeInternal().
1 parent 0664c1a commit 3a939ff

File tree

7 files changed

+133
-71
lines changed

7 files changed

+133
-71
lines changed

‎Include/internal/pycore_bytesobject.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ extern PyObject* _PyBytes_FromHex(
2020

2121
// Helper for PyBytes_DecodeEscape that detects invalid escape chars.
2222
// Export for test_peg_generator.
23-
PyAPI_FUNC(PyObject*) _PyBytes_DecodeEscape(const char *, Py_ssize_t,
24-
const char *, const char **);
23+
PyAPI_FUNC(PyObject*) _PyBytes_DecodeEscape2(const char *, Py_ssize_t,
24+
const char *, int *);
2525

2626

2727
// Substring Search.

‎Include/internal/pycore_unicodeobject.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -141,14 +141,14 @@ extern PyObject* _PyUnicode_DecodeUnicodeEscapeStateful(
141141
// Helper for PyUnicode_DecodeUnicodeEscape that detects invalid escape
142142
// chars.
143143
// Export for test_peg_generator.
144-
PyAPI_FUNC(PyObject*) _PyUnicode_DecodeUnicodeEscapeInternal(
144+
PyAPI_FUNC(PyObject*) _PyUnicode_DecodeUnicodeEscapeInternal2(
145145
const char *string, /* Unicode-Escape encoded string */
146146
Py_ssize_t length, /* size of string */
147147
const char *errors, /* error handling */
148148
Py_ssize_t *consumed, /* bytes consumed */
149-
const char **first_invalid_escape); /* on return, points to first
150-
invalid escaped char in
151-
string. */
149+
int *first_invalid_escape); /* on return, if not -1, contain the first
150+
invalid escaped char (<= 0xff) or invalid
151+
octal escape (> 0xff) in string. */
152152

153153
/* --- Raw-Unicode-Escape Codecs ---------------------------------------------- */
154154

‎Lib/test/test_codeccallbacks.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import codecs
33
import html.entities
44
import itertools
5+
import re
56
import sys
67
import unicodedata
78
import unittest
@@ -1125,7 +1126,7 @@ def test_bug828737(self):
11251126
text = 'abc<def>ghi'*n
11261127
text.translate(charmap)
11271128

1128-
def test_mutatingdecodehandler(self):
1129+
def test_mutating_decode_handler(self):
11291130
baddata = [
11301131
("ascii", b"\xff"),
11311132
("utf-7", b"++"),
@@ -1160,6 +1161,42 @@ def mutating(exc):
11601161
for (encoding, data) in baddata:
11611162
self.assertEqual(data.decode(encoding, "test.mutating"), "\u4242")
11621163

1164+
def test_mutating_decode_handler_unicode_escape(self):
1165+
decode = codecs.unicode_escape_decode
1166+
def mutating(exc):
1167+
if isinstance(exc, UnicodeDecodeError):
1168+
r = data.get(exc.object[:exc.end])
1169+
if r is not None:
1170+
exc.object = r[0] + exc.object[exc.end:]
1171+
return ('\u0404', r[1])
1172+
raise AssertionError("don't know how to handle %r" % exc)
1173+
1174+
codecs.register_error('test.mutating2', mutating)
1175+
data = {
1176+
br'\x0': (b'\\', 0),
1177+
br'\x3': (b'xxx\\', 3),
1178+
br'\x5': (b'x\\', 1),
1179+
}
1180+
def check(input, expected, msg):
1181+
with self.assertWarns(DeprecationWarning) as cm:
1182+
self.assertEqual(decode(input, 'test.mutating2'), (expected, len(input)))
1183+
self.assertIn(msg, str(cm.warning))
1184+
1185+
check(br'\x0n\z', '\u0404\n\\z', r'"\z" is an invalid escape sequence')
1186+
check(br'\x0n\501', '\u0404\n\u0141', r'"\501" is an invalid octal escape sequence')
1187+
check(br'\x0z', '\u0404\\z', r'"\z" is an invalid escape sequence')
1188+
1189+
check(br'\x3n\zr', '\u0404\n\\zr', r'"\z" is an invalid escape sequence')
1190+
check(br'\x3zr', '\u0404\\zr', r'"\z" is an invalid escape sequence')
1191+
check(br'\x3z5', '\u0404\\z5', r'"\z" is an invalid escape sequence')
1192+
check(memoryview(br'\x3z5x')[:-1], '\u0404\\z5', r'"\z" is an invalid escape sequence')
1193+
check(memoryview(br'\x3z5xy')[:-2], '\u0404\\z5', r'"\z" is an invalid escape sequence')
1194+
1195+
check(br'\x5n\z', '\u0404\n\\z', r'"\z" is an invalid escape sequence')
1196+
check(br'\x5n\501', '\u0404\n\u0141', r'"\501" is an invalid octal escape sequence')
1197+
check(br'\x5z', '\u0404\\z', r'"\z" is an invalid escape sequence')
1198+
check(memoryview(br'\x5zy')[:-1], '\u0404\\z', r'"\z" is an invalid escape sequence')
1199+
11631200
# issue32583
11641201
def test_crashing_decode_handler(self):
11651202
# better generating one more character to fill the extra space slot

‎Lib/test/test_codecs.py‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,23 +1196,39 @@ def test_escape(self):
11961196
check(br"[\1010]", b"[A0]")
11971197
check(br"[\x41]", b"[A]")
11981198
check(br"[\x410]", b"[A0]")
1199+
1200+
def test_warnings(self):
1201+
decode = codecs.escape_decode
1202+
check = coding_checker(self, decode)
11991203
for i in range(97, 123):
12001204
b = bytes([i])
12011205
if b not in b'abfnrtvx':
1202-
with self.assertWarns(DeprecationWarning):
1206+
with self.assertWarnsRegex(DeprecationWarning,
1207+
r'"\\%c" is an invalid escape sequence' % i):
12031208
check(b"\\" + b, b"\\" + b)
1204-
with self.assertWarns(DeprecationWarning):
1209+
with self.assertWarnsRegex(DeprecationWarning,
1210+
r'"\\%c" is an invalid escape sequence' % (i-32)):
12051211
check(b"\\" + b.upper(), b"\\" + b.upper())
1206-
with self.assertWarns(DeprecationWarning):
1212+
with self.assertWarnsRegex(DeprecationWarning,
1213+
r'"\\8" is an invalid escape sequence'):
12071214
check(br"\8", b"\\8")
12081215
with self.assertWarns(DeprecationWarning):
12091216
check(br"\9", b"\\9")
1210-
with self.assertWarns(DeprecationWarning):
1217+
with self.assertWarnsRegex(DeprecationWarning,
1218+
r'"\\\xfa" is an invalid escape sequence') as cm:
12111219
check(b"\\\xfa", b"\\\xfa")
12121220
for i in range(0o400, 0o1000):
1213-
with self.assertWarns(DeprecationWarning):
1221+
with self.assertWarnsRegex(DeprecationWarning,
1222+
r'"\\%o" is an invalid octal escape sequence' % i):
12141223
check(rb'\%o' % i, bytes([i & 0o377]))
12151224

1225+
with self.assertWarnsRegex(DeprecationWarning,
1226+
r'"\\z" is an invalid escape sequence'):
1227+
self.assertEqual(decode(br'\x\z', 'ignore'), (b'\\z', 4))
1228+
with self.assertWarnsRegex(DeprecationWarning,
1229+
r'"\\501" is an invalid octal escape sequence'):
1230+
self.assertEqual(decode(br'\x\501', 'ignore'), (b'A', 6))
1231+
12161232
def test_errors(self):
12171233
decode = codecs.escape_decode
12181234
self.assertRaises(ValueError, decode, br"\x")
@@ -2661,24 +2677,40 @@ def test_escape_decode(self):
26612677
check(br"[\x410]", "[A0]")
26622678
check(br"\u20ac", "\u20ac")
26632679
check(br"\U0001d120", "\U0001d120")
2680+
2681+
def test_decode_warnings(self):
2682+
decode = codecs.unicode_escape_decode
2683+
check = coding_checker(self, decode)
26642684
for i in range(97, 123):
26652685
b = bytes([i])
26662686
if b not in b'abfnrtuvx':
2667-
with self.assertWarns(DeprecationWarning):
2687+
with self.assertWarnsRegex(DeprecationWarning,
2688+
r'"\\%c" is an invalid escape sequence' % i):
26682689
check(b"\\" + b, "\\" + chr(i))
26692690
if b.upper() not in b'UN':
2670-
with self.assertWarns(DeprecationWarning):
2691+
with self.assertWarnsRegex(DeprecationWarning,
2692+
r'"\\%c" is an invalid escape sequence' % (i-32)):
26712693
check(b"\\" + b.upper(), "\\" + chr(i-32))
2672-
with self.assertWarns(DeprecationWarning):
2694+
with self.assertWarnsRegex(DeprecationWarning,
2695+
r'"\\8" is an invalid escape sequence'):
26732696
check(br"\8", "\\8")
26742697
with self.assertWarns(DeprecationWarning):
26752698
check(br"\9", "\\9")
2676-
with self.assertWarns(DeprecationWarning):
2699+
with self.assertWarnsRegex(DeprecationWarning,
2700+
r'"\\\xfa" is an invalid escape sequence') as cm:
26772701
check(b"\\\xfa", "\\\xfa")
26782702
for i in range(0o400, 0o1000):
2679-
with self.assertWarns(DeprecationWarning):
2703+
with self.assertWarnsRegex(DeprecationWarning,
2704+
r'"\\%o" is an invalid octal escape sequence' % i):
26802705
check(rb'\%o' % i, chr(i))
26812706

2707+
with self.assertWarnsRegex(DeprecationWarning,
2708+
r'"\\z" is an invalid escape sequence'):
2709+
self.assertEqual(decode(br'\x\z', 'ignore'), ('\\z', 4))
2710+
with self.assertWarnsRegex(DeprecationWarning,
2711+
r'"\\501" is an invalid octal escape sequence'):
2712+
self.assertEqual(decode(br'\x\501', 'ignore'), ('\u0141', 6))
2713+
26822714
def test_decode_errors(self):
26832715
decode = codecs.unicode_escape_decode
26842716
for c, d in (b'x', 2), (b'u', 4), (b'U', 4):

‎Objects/bytesobject.c‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1076,10 +1076,10 @@ _PyBytes_FormatEx(const char *format, Py_ssize_t format_len,
10761076
}
10771077

10781078
/* Unescape a backslash-escaped string. */
1079-
PyObject *_PyBytes_DecodeEscape(const char *s,
1079+
PyObject *_PyBytes_DecodeEscape2(const char *s,
10801080
Py_ssize_t len,
10811081
const char *errors,
1082-
const char **first_invalid_escape)
1082+
int *first_invalid_escape)
10831083
{
10841084
int c;
10851085
char *p;
@@ -1093,7 +1093,7 @@ PyObject *_PyBytes_DecodeEscape(const char *s,
10931093
return NULL;
10941094
writer.overallocate = 1;
10951095

1096-
*first_invalid_escape = NULL;
1096+
*first_invalid_escape = -1;
10971097

10981098
end = s + len;
10991099
while (s < end) {
@@ -1131,9 +1131,8 @@ PyObject *_PyBytes_DecodeEscape(const char *s,
11311131
c = (c<<3) + *s++ - '0';
11321132
}
11331133
if (c > 0377) {
1134-
if (*first_invalid_escape == NULL) {
1135-
*first_invalid_escape = s-3; /* Back up 3 chars, since we've
1136-
already incremented s. */
1134+
if (*first_invalid_escape == -1) {
1135+
*first_invalid_escape = c;
11371136
}
11381137
}
11391138
*p++ = c;
@@ -1174,9 +1173,8 @@ PyObject *_PyBytes_DecodeEscape(const char *s,
11741173
break;
11751174

11761175
default:
1177-
if (*first_invalid_escape == NULL) {
1178-
*first_invalid_escape = s-1; /* Back up one char, since we've
1179-
already incremented s. */
1176+
if (*first_invalid_escape == -1) {
1177+
*first_invalid_escape = (unsigned char)s[-1];
11801178
}
11811179
*p++ = '\\';
11821180
s--;
@@ -1196,16 +1194,15 @@ PyObject *PyBytes_DecodeEscape(const char *s,
11961194
Py_ssize_t Py_UNUSED(unicode),
11971195
const char *Py_UNUSED(recode_encoding))
11981196
{
1199-
const char* first_invalid_escape;
1200-
PyObject *result = _PyBytes_DecodeEscape(s, len, errors,
1197+
int first_invalid_escape;
1198+
PyObject *result = _PyBytes_DecodeEscape2(s, len, errors,
12011199
&first_invalid_escape);
12021200
if (result == NULL)
12031201
return NULL;
1204-
if (first_invalid_escape != NULL) {
1205-
unsigned char c = *first_invalid_escape;
1206-
if ('4' <= c && c <= '7') {
1202+
if (first_invalid_escape != -1) {
1203+
if (first_invalid_escape > 0xff) {
12071204
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
1208-
"b\"\\%.3s\" is an invalid octal escape sequence. "
1205+
"b\"\\%o\" is an invalid octal escape sequence. "
12091206
"Such sequences will not work in the future. ",
12101207
first_invalid_escape) < 0)
12111208
{
@@ -1217,7 +1214,7 @@ PyObject *PyBytes_DecodeEscape(const char *s,
12171214
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
12181215
"b\"\\%c\" is an invalid escape sequence. "
12191216
"Such sequences will not work in the future. ",
1220-
c) < 0)
1217+
first_invalid_escape) < 0)
12211218
{
12221219
Py_DECREF(result);
12231220
return NULL;

‎Objects/unicodeobject.c‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6599,11 +6599,11 @@ _PyUnicode_GetNameCAPI(void)
65996599
/* --- Unicode Escape Codec ----------------------------------------------- */
66006600

66016601
PyObject *
6602-
_PyUnicode_DecodeUnicodeEscapeInternal(const char *s,
6602+
_PyUnicode_DecodeUnicodeEscapeInternal2(const char *s,
66036603
Py_ssize_t size,
66046604
const char *errors,
66056605
Py_ssize_t *consumed,
6606-
const char **first_invalid_escape)
6606+
int *first_invalid_escape)
66076607
{
66086608
const char *starts = s;
66096609
_PyUnicodeWriter writer;
@@ -6613,7 +6613,7 @@ _PyUnicode_DecodeUnicodeEscapeInternal(const char *s,
66136613
_PyUnicode_Name_CAPI *ucnhash_capi;
66146614

66156615
// so we can remember if we've seen an invalid escape char or not
6616-
*first_invalid_escape = NULL;
6616+
*first_invalid_escape = -1;
66176617

66186618
if (size == 0) {
66196619
if (consumed) {
@@ -6701,9 +6701,8 @@ _PyUnicode_DecodeUnicodeEscapeInternal(const char *s,
67016701
}
67026702
}
67036703
if (ch > 0377) {
6704-
if (*first_invalid_escape == NULL) {
6705-
*first_invalid_escape = s-3; /* Back up 3 chars, since we've
6706-
already incremented s. */
6704+
if (*first_invalid_escape == -1) {
6705+
*first_invalid_escape = ch;
67076706
}
67086707
}
67096708
WRITE_CHAR(ch);
@@ -6798,9 +6797,8 @@ _PyUnicode_DecodeUnicodeEscapeInternal(const char *s,
67986797
goto error;
67996798

68006799
default:
6801-
if (*first_invalid_escape == NULL) {
6802-
*first_invalid_escape = s-1; /* Back up one char, since we've
6803-
already incremented s. */
6800+
if (*first_invalid_escape == -1) {
6801+
*first_invalid_escape = c;
68046802
}
68056803
WRITE_ASCII_CHAR('\\');
68066804
WRITE_CHAR(c);
@@ -6845,17 +6843,16 @@ _PyUnicode_DecodeUnicodeEscapeStateful(const char *s,
68456843
const char *errors,
68466844
Py_ssize_t *consumed)
68476845
{
6848-
const char *first_invalid_escape;
6849-
PyObject *result = _PyUnicode_DecodeUnicodeEscapeInternal(s, size, errors,
6846+
int first_invalid_escape;
6847+
PyObject *result = _PyUnicode_DecodeUnicodeEscapeInternal2(s, size, errors,
68506848
consumed,
68516849
&first_invalid_escape);
68526850
if (result == NULL)
68536851
return NULL;
6854-
if (first_invalid_escape != NULL) {
6855-
unsigned char c = *first_invalid_escape;
6856-
if ('4' <= c && c <= '7') {
6852+
if (first_invalid_escape != -1) {
6853+
if (first_invalid_escape > 0xff) {
68576854
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
6858-
"\"\\%.3s\" is an invalid octal escape sequence. "
6855+
"\"\\%o\" is an invalid octal escape sequence. "
68596856
"Such sequences will not work in the future. ",
68606857
first_invalid_escape) < 0)
68616858
{
@@ -6867,7 +6864,7 @@ _PyUnicode_DecodeUnicodeEscapeStateful(const char *s,
68676864
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
68686865
"\"\\%c\" is an invalid escape sequence. "
68696866
"Such sequences will not work in the future. ",
6870-
c) < 0)
6867+
first_invalid_escape) < 0)
68716868
{
68726869
Py_DECREF(result);
68736870
return NULL;

0 commit comments

Comments
 (0)