Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible divide by zero problems #88234

Closed
yiyuaner mannequin opened this issue May 7, 2021 · 3 comments
Closed

Possible divide by zero problems #88234

yiyuaner mannequin opened this issue May 7, 2021 · 3 comments
Labels
3.11 only security fixes topic-unicode type-bug An unexpected behavior, bug, or error

Comments

@yiyuaner
Copy link
Mannequin

yiyuaner mannequin commented May 7, 2021

BPO 44068
Nosy @vstinner, @ezio-melotti, @methane, @serhiy-storchaka, @yiyuaner

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-05-10.20:53:17.884>
created_at = <Date 2021-05-07.15:00:44.871>
labels = ['type-bug', 'invalid', 'expert-unicode', '3.11']
title = 'Possible divide by zero problems'
updated_at = <Date 2021-05-10.20:53:17.880>
user = 'https://github.com/yiyuaner'

bugs.python.org fields:

activity = <Date 2021-05-10.20:53:17.880>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2021-05-10.20:53:17.884>
closer = 'vstinner'
components = ['Unicode']
creation = <Date 2021-05-07.15:00:44.871>
creator = 'yiyuaner'
dependencies = []
files = []
hgrepos = []
issue_num = 44068
keywords = []
message_count = 3.0
messages = ['393188', '393189', '393431']
nosy_count = 5.0
nosy_names = ['vstinner', 'ezio.melotti', 'methane', 'serhiy.storchaka', 'yiyuaner']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue44068'
versions = ['Python 3.11']

@yiyuaner
Copy link
Mannequin Author

yiyuaner mannequin commented May 7, 2021

In the file Objects/unicodeobject.c, we have the following code:

static PyObject*
resize_compact(PyObject *unicode, Py_ssize_t length) {
  ...
  char_size = PyUnicode_KIND(unicode);
  ...
  if (length > ((PY_SSIZE_T_MAX - struct_size) / char_size - 1)) {
    PyErr_NoMemory();
    return NULL;
  }
}

However, PyUnicode_KIND may return 0 if the variable unicode has kind PyUnicode_WCHAR_KIND, leading to a divide by zero problem.

The same pattern is also used without checking in function "static int
resize_inplace(PyObject *unicode, Py_ssize_t length)".

Here is the link to the code location: https://github.com/python/cpython/blob/main/Objects/unicodeobject.c#L1045

Should we add an explicit check on variable char_size before using it in division?

@yiyuaner yiyuaner mannequin added 3.11 only security fixes topic-unicode type-bug An unexpected behavior, bug, or error labels May 7, 2021
@serhiy-storchaka
Copy link
Member

It cannot be 0 if PyUnicode_IS_READY() returns true.

@vstinner
Copy link
Member

Should we add an explicit check on variable char_size before using it in division?

Search for "There are 4 forms of Unicode strings" in Include/cpython/unicodeobject.h. char_size cannot be 0 in resize_compact(): it's checked by 2 assertions:

assert(PyUnicode_IS_READY(unicode));
assert(PyUnicode_IS_COMPACT(unicode));

The function cannot be called on a string which is not compact.

There is no bug, I close the issue.

Hopefully, Inada-san will remove state.compact field in Python 3.12 with his PEP-623!
https://www.python.org/dev/peps/pep-0623/#python-3-12

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes topic-unicode type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants