[THRIFT-3941] WinXP version of thrift_poll() relies on undefined behavior by passing a destructed variable to select()

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.9.3
• Fix Version/s: 0.10.0
• Component/s: C++ - Library
• Assignee: Ted Wang
• Reporter: Ted Wang
• Created: 04/Oct/16 15:56
• Updated: 11/Oct/16 01:12
• Resolved: 05/Oct/16 10:33

Description

thrift_poll() for WINVER <= 0x0502 in thrift/windows/WinFnctl.cpp shadows the 'time_out' variable, and it ends up passing the destructed copy to select():

timeval time_out;
timeval* time_out_ptr = NULL;
if (timeout >= 0) {
timeval time_out =

{timeout / 1000, (timeout % 1000) * 1000}

;
time_out_ptr = &time_out;
} else

{ // to avoid compiler warnings (void)time_out; (void)timeout; }

int sktready = select(1, read_fds_ptr, write_fds_ptr, NULL, time_out_ptr);

Stepping through this code in the debugger, it looks like MSVC reserves a large enough stack frame to avoid overwriting the variable when calling select(), which may be why this hasn't been caught yet.

Issue Links

• links to: GitHub Pull Request #1107