Skip to content

[2.7] bpo-36337: Use ssize_t and size_t for socket.send()/sendall() #12397

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.

Sign up for GitHub

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

Merged
vstinner merged 11 commits into from
Mar 19, 2019
Merged

[2.7] bpo-36337: Use ssize_t and size_t for socket.send()/sendall() #12397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
17bf06c
bpo-36337: Use ssize_t and size_t for socket.send()/sendall()
matrixise Mar 18, 2019
4c59062
On Windows, the type of send() length argument is int, not size_t: ba…
matrixise Mar 18, 2019
59c20ea
Fix on Windows
matrixise Mar 18, 2019
c78ef94
Fix PEP7
matrixise Mar 18, 2019
b4e16ea
convert to long from ssize_t
matrixise Mar 18, 2019
405a5d2
On VMS, sendsegmented(), len type is int, not Py_ssize_t
matrixise Mar 18, 2019
995c41d
Improve the blurb entry
matrixise Mar 18, 2019
3300ad6
avoid a buffer overflow when len is greater than INT_MAX on VMS
matrixise Mar 18, 2019
55f2b36
use Py_ssize_t for sendsegmented
matrixise Mar 18, 2019
40dbaf4
Improvement with Victor
matrixise Mar 18, 2019
84a8d9f
Improvement with the remark of Serhiy
matrixise Mar 18, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2019-03-18-10-08-30.bpo-36337.QhJnXy.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix buffer overflow in :meth:`~socket.socket.send` and
:meth:`~socket.socket.sendall` methods of :func:`socket.socket` for data larger
than 2 GiB.
29 changes: 21 additions & 8 deletions Modules/socketmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,16 +625,16 @@ set_gaierror(int error)

#ifdef __VMS
/* Function to send in segments */
static int
sendsegmented(int sock_fd, char *buf, int len, int flags)
static Py_ssize_t
sendsegmented(int sock_fd, char *buf, Py_ssize_t len, int flags)
{
int n = 0;
int remaining = len;
Py_ssize_t remaining = len;

while (remaining > 0) {
unsigned int segment;

segment = (remaining >= SEGMENT_SIZE ? SEGMENT_SIZE : remaining);
segment = ((size_t)remaining >= SEGMENT_SIZE ? SEGMENT_SIZE : (unsigned int) remaining);
Copy link
Copy Markdown
Member

@pablogsal pablogsal Mar 18, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, is this cast safe? In C99 ssize_t can be an unsigned long, unless I am missing something. I think you need:

Py_SAFE_DOWNCAST(remaining, Py_ssize_t, unsigned int)

Copy link
Copy Markdown
Member

@vstinner vstinner Mar 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SEGMENT_SIZE is 32 KiB: remaining is only casted for values between [0; 32 KiB]:

#define SEGMENT_SIZE (32 * 1024 -1)

n = send(sock_fd, buf, segment, flags);
if (n < 0) {
return n;
Expand Down Expand Up @@ -2797,7 +2797,8 @@ static PyObject *
sock_send(PySocketSockObject *s, PyObject *args)
{
char *buf;
int len, n = -1, flags = 0, timeout;
int flags = 0, timeout;
Py_ssize_t len, n = -1;
Copy link
Copy Markdown
Member

@vstinner vstinner Mar 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return PyInt_FromLong((long)n); below is wrong

Py_buffer pbuf;

if (!PyArg_ParseTuple(args, "s*|i:send", &pbuf, &flags))
Expand All @@ -2813,12 +2814,18 @@ sock_send(PySocketSockObject *s, PyObject *args)
BEGIN_SELECT_LOOP(s)
Py_BEGIN_ALLOW_THREADS
timeout = internal_select_ex(s, 1, interval);
if (!timeout)
if (!timeout) {
#ifdef __VMS
n = sendsegmented(s->sock_fd, buf, len, flags);
Copy link
Copy Markdown
Member

@vstinner vstinner Mar 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's wrong: sendsegmented() len type is int, not Py_ssize_t. Don't forget the VMS! :-)

#elif defined(MS_WINDOWS)
if (len > INT_MAX) {
len = INT_MAX;
}
n = send(s->sock_fd, buf, (int)len, flags);
#else
n = send(s->sock_fd, buf, len, flags);
#endif
}
Py_END_ALLOW_THREADS
if (timeout == 1) {
PyBuffer_Release(&pbuf);
Expand All @@ -2830,7 +2837,7 @@ sock_send(PySocketSockObject *s, PyObject *args)
PyBuffer_Release(&pbuf);
if (n < 0)
return s->errorhandler();
return PyInt_FromLong((long)n);
return PyInt_FromSsize_t(n);
}

PyDoc_STRVAR(send_doc,
Expand All @@ -2847,7 +2854,8 @@ static PyObject *
sock_sendall(PySocketSockObject *s, PyObject *args)
{
char *buf;
int len, n = -1, flags = 0, timeout, saved_errno;
int flags = 0, timeout, saved_errno;
Py_ssize_t len, n = -1;
Py_buffer pbuf;

if (!PyArg_ParseTuple(args, "s*|i:sendall", &pbuf, &flags))
Expand All @@ -2868,6 +2876,11 @@ sock_sendall(PySocketSockObject *s, PyObject *args)
if (!timeout) {
#ifdef __VMS
n = sendsegmented(s->sock_fd, buf, len, flags);
#elif defined(MS_WINDOWS)
if (len > INT_MAX) {
len = INT_MAX;
}
n = send(s->sock_fd, buf, (int)len, flags);
#else
n = send(s->sock_fd, buf, len, flags);
#endif
Expand Down