Found a little problem with the sip request handling that could lead to a quick crash of asterisk, and a road to a DOS attack if left unfixed.
Attaching to a running asterisk with "telnet hostname 5060", I would input "something", then hit return three times, and asterisk crashes. I traced it to handle_request_do(), which zeroes out the data (an ast_str ptr) if the string is too short. Instead of freeing the struct and nulling the pointer, it now just resets it, because this ast_str is expected by the calling routine to still be there after handle_request_do() returns. This appears to fix the crash. I assume that it was introduced with ast_str's being adopted. It's a subtle and easy-to-miss sort of problem. I also found all the places where the req.data is freed, and made sure the ptr is Nulled out as well; no good leaving bad ptrs laying around-- I didn't need to do this, but it seemed a good thing to do... git-svn-id: http://svn.digium.com/svn/asterisk/trunk@112874 f38db490-d61c-443f-a65b-d21fe96a405b
This commit is contained in:
parent
f3fb6465c2
commit
c7dcc10a2d
|
@ -2252,8 +2252,11 @@ cleanup2:
|
|||
ser = ast_tcptls_session_instance_destroy(ser);
|
||||
if (reqcpy.data)
|
||||
ast_free(reqcpy.data);
|
||||
if (req.data)
|
||||
if (req.data) {
|
||||
ast_free(req.data);
|
||||
req.data = NULL;
|
||||
}
|
||||
|
||||
|
||||
if (req.socket.lock) {
|
||||
ast_mutex_destroy(req.socket.lock);
|
||||
|
@ -18129,8 +18132,10 @@ static int sipsock_read(int *id, int fd, short events, void *ignore)
|
|||
req.socket.lock = NULL;
|
||||
|
||||
handle_request_do(&req, &sin);
|
||||
if (req.data)
|
||||
if (req.data) {
|
||||
ast_free(req.data);
|
||||
req.data = NULL;
|
||||
}
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
@ -18159,8 +18164,7 @@ static int handle_request_do(struct sip_request *req, struct sockaddr_in *sin)
|
|||
ast_verbose("--- (%d headers %d lines)%s ---\n", req->headers, req->lines, (req->headers + req->lines == 0) ? " Nat keepalive" : "");
|
||||
|
||||
if (req->headers < 2) { /* Must have at least two headers */
|
||||
ast_free(req->data);
|
||||
req->data = NULL;
|
||||
ast_str_reset(req->data); /* nulling this out is NOT a good idea here. */
|
||||
return 1;
|
||||
}
|
||||
|
||||
|
|
Reference in New Issue