AnsweredAssumed Answered

Potential Bug in httpsrv_readreq() in RTCS for MQX for KSDK 1.1

Question asked by Bowe Neuenschwander on May 21, 2015
Latest reply on Jul 7, 2015 by Bowe Neuenschwander

We are using MQX for KDSK 1.1.  In looking at httpsrv_readreq() in httpsrv_task.c, it seems that there is a potential bug that might occur if the the buffer is completely filled at line 577 because line 602 might not find a '\n' in the buffer and continue reading past it.  This bug would be exploited in the case that an HTTP request header has a line that is larger than the buffer, which will probably never happen unless intentionally exploited.  I have not experimentally proven this bug, so maybe there is some way I am not seeing that prevents it.

 

Here is line 577:

read = httpsrv_recv(session, session->buffer.data+session->buffer.offset, HTTPSRV_SES_BUF_SIZE_PRV-session->buffer.offset, 0);

 

Here is line 602:

while (session->request.process_header && ((line_end = strchr(line_start, '\n')) != NULL))

 

The fix we implemented until we move to MQX for KSDK 1.2 is to ensure the buffer is never completely filled (and then null-terminating).  This is done by changing line 577, as shown below, and leaving line 602 alone.  The second line in this fix is probably not necessary since the free buffer space is zero-ed in line 653.

read = httpsrv_recv(session, session->buffer.data+session->buffer.offset, HTTPSRV_SES_BUF_SIZE_PRV-session->buffer.offset-1, 0);
if(read > 0) session->buffer.data[session->buffer.offset+read] = '\0';

 

Again, recommendations/comments are welcome.

 

-Bowe

 

NOTE:  In MQX for KSDK 1.2, it looks like this function has been renamed to httpsrv_req_read(), and the potential bug is fixed by using memchr(), which takes a length.  So this potential problem is probably already known.

Outcomes