Possible loss of large text data in `_curses.window.{instr,getstr}`

[picnixz]

Bug report

Bug description:

_curses.window.instr is meant to extract a string of characters between two positions and we have a maximum number of allowed characters which is 1023. However, this limit is not enforced, namely we do the following:

winnstr(self->win, rtn, Py_MIN(n, 1023));

IOW, we cannot return more than 1023 characters in a single API call. This should be documented and enforced at runtime, so that users may know that they need multiple API calls, or we should allocate heap memory instead (currently the buffer holding the output is allocated on the stack).

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

• gh-134209: use heap-allocated memory in _curses.window.{instr,getstr} #134283
• [3.14] gh-134209: use heap-allocated memory in _curses.window.{instr,getstr} (GH-134283) #134391

[zydtiger]

I am a new sprinter and I am looking into the C code! I have an idea of using the following approach:

rtn = PyMem_Malloc(n);
if (!rtn) {
  return PyErr_NoMemory();
}
...
PyObject *obj = PyBytes_FromString(rtn);
PyMem_Free(rtn);
return obj;

I feel that simply allocating some random size on heap may be dangerous, should this be ok?

[picnixz]

Thanks for looking at this one. Indeed, it would be dangerous because someone can cause a DoS. So, instead, we should either allocate by chunks, or explicitly state that at most 1023 characters will be read.

Your solution is essentially what I had in mind but ideally I think it's easier to maintain if we just document that reading more than 1023 characters is not supported and that we will truncate n on our side.

I don't know if it's better to raise an exception or not, but maybe. For now, no one has ever complained about this and I only stumbled upon it by chance.

@encukou do you prefer allocating the buffer chunk by chunk, until curses tell us "I can't read more" or do you prefer to just reject very large texts to read?

[gpshead]

we're going to make it allocate a buffer in the sprint. it doesn't feel like people are hitting the limit yet so we'll just go with allocating a larger one and documenting the limit for starters.