Fix potential buffer overrun and memory leak when executing "sip show peers"

The "sip show peers" command uses a fix sized array to sort the current peers
in the peers ao2_container.  The size of the array is based on the current
number of peers in the container.  However, once the size of the array is
determined, the number of peers in the container can change, as the peers
container is not locked.  This could cause a buffer overrun when populating
the array, if peers were added to the container after the array was created.
Additionally, a memory leak of the allocated array would occur if a user
caused the _show_peers method to return CLI_SHOWUSAGE.

We now create a snapshot of the current peers using an ao2_callback with the
OBJ_MULTIPLE flag.  This size of the array is set to the number of peers
that the iterator will iterate over; hence, if peers are added or removed
from the peers container it will not affect the execution of the "sip show
peers" command.

Review: https://reviewboard.asterisk.org/r/1738/

(closes issue ASTERISK-19231)
(closes issue ASTERISK-19361)
Reported by: Thomas Arimont, Jamuel Starkey
Tested by: Thomas Arimont, Jamuel Starkey
Patches: sip_show_peers_2012_02_16.diff uploaded by mjordan (license 6283)




git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@356214 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Matthew Jordan
2012-02-22 14:50:20 +00:00
parent e880b4a205
commit 6453352768

View File

@@ -16644,7 +16644,7 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
regex_t regexbuf; regex_t regexbuf;
int havepattern = FALSE; int havepattern = FALSE;
struct sip_peer *peer; struct sip_peer *peer;
struct ao2_iterator i; struct ao2_iterator* it_peers;
/* the last argument is left-aligned, so we don't need a size anyways */ /* the last argument is left-aligned, so we don't need a size anyways */
#define FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-3.3s %-8s %-10s %s\n" #define FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-3.3s %-8s %-10s %s\n"
@@ -16658,12 +16658,10 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
const char *id; const char *id;
char idtext[256] = ""; char idtext[256] = "";
int realtimepeers; int realtimepeers;
int objcount = ao2_container_count(peers);
struct sip_peer **peerarray; struct sip_peer **peerarray;
int k; int k;
realtimepeers = ast_check_realtime("sippeers"); realtimepeers = ast_check_realtime("sippeers");
peerarray = ast_calloc(sizeof(struct sip_peer *), objcount);
if (s) { /* Manager - get ActionID */ if (s) { /* Manager - get ActionID */
id = astman_get_header(m, "ActionID"); id = astman_get_header(m, "ActionID");
@@ -16685,11 +16683,26 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
return CLI_SHOWUSAGE; return CLI_SHOWUSAGE;
} }
if (!s) /* Normal list */ if (!s) {
/* Normal list */
ast_cli(fd, FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "ACL", "Port", "Status", (realtimepeers ? "Realtime" : "")); ast_cli(fd, FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "ACL", "Port", "Status", (realtimepeers ? "Realtime" : ""));
}
i = ao2_iterator_init(peers, 0); ao2_lock(peers);
while ((peer = ao2_t_iterator_next(&i, "iterate thru peers table"))) { if (!(it_peers = ao2_callback(peers, OBJ_MULTIPLE, NULL, NULL))) {
ast_log(AST_LOG_ERROR, "Unable to create iterator for peers container for sip show peers\n");
ao2_unlock(peers);
return CLI_FAILURE;
}
if (!(peerarray = ast_calloc(sizeof(struct sip_peer *), ao2_container_count(peers)))) {
ast_log(AST_LOG_ERROR, "Unable to allocate peer array for sip show peers\n");
ao2_iterator_destroy(it_peers);
ao2_unlock(peers);
return CLI_FAILURE;
}
ao2_unlock(peers);
while ((peer = ao2_t_iterator_next(it_peers, "iterate thru peers table"))) {
ao2_lock(peer); ao2_lock(peer);
if (!(peer->type & SIP_TYPE_PEER)) { if (!(peer->type & SIP_TYPE_PEER)) {
@@ -16699,7 +16712,6 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
} }
if (havepattern && regexec(&regexbuf, peer->name, 0, NULL, 0)) { if (havepattern && regexec(&regexbuf, peer->name, 0, NULL, 0)) {
objcount--;
ao2_unlock(peer); ao2_unlock(peer);
unref_peer(peer, "toss iterator peer ptr before continue"); unref_peer(peer, "toss iterator peer ptr before continue");
continue; continue;
@@ -16708,7 +16720,7 @@ static char *_sip_show_peers(int fd, int *total, struct mansession *s, const str
peerarray[total_peers++] = peer; peerarray[total_peers++] = peer;
ao2_unlock(peer); ao2_unlock(peer);
} }
ao2_iterator_destroy(&i); ao2_iterator_destroy(it_peers);
qsort(peerarray, total_peers, sizeof(struct sip_peer *), peercomparefunc); qsort(peerarray, total_peers, sizeof(struct sip_peer *), peercomparefunc);