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

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

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?

@encukou

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?

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.

gh-134209: use heap-allocated memory in `_curses.window.{instr,getstr…

pythongh-134209: use heap-allocated memory in `_curses.window.{instr,…

[3.14] gh-134209: use heap-allocated memory in `_curses.window.{instr…

Alright, significantly improved code via those PRs. Thanks & congratz on the first time pyconus sprint PR!

Maybe open issue:

Do we need to deal with larger buffers here? meaning we'd need to loop? nobody has asked so... (is an assumption of "realistically no normal user ever has a terminal window 2047 bytes wide?" a bad one?)
What about invalid values of n being passed in? are we treating those as ValueError? should we start? the truncation is silent right now which is gross... but the curses C APIs are gross...

I can imagine someone with really large monitors which may have some realtime graphs diaplayed on it but it'll likely be a webapp rather than a Python one.

For invalid ones, we could start emitting a warning and later we would turn that warning into an exception (later = 3.17)

Note: I closed the issue because I'd like a new one if we were to make dynamic allocations and/or warns the user as it's a user-facing change that we don't want to backport.

pythongh-134209: use heap-allocated memory in `_curses.window.{instr,…

Navigation Menu

Repository navigation

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

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

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

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

zydtiger commented on May 19, 2025

picnixz commented on May 19, 2025

gpshead commented on May 19, 2025

gpshead commented on May 20, 2025

picnixz commented on May 22, 2025

picnixz commented on May 24, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

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