Skip to content

Commit 76afe34

Browse files
committed
imc: fix member list buffer overflow
Build #list replies in an exact-sized pkg buffer instead of the fixed module buffer. Check length arithmetic before copying member URIs so large rooms cannot overflow the response body.
1 parent e95fbff commit 76afe34

1 file changed

Lines changed: 46 additions & 7 deletions

File tree

modules/imc/imc_cmd.c

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include <string.h>
2626
#include <stdlib.h>
2727
#include <unistd.h>
28+
#include <limits.h>
2829
#include <sys/types.h>
2930
#include "../../mem/shm_mem.h"
3031
#include "../../mem/mem.h"
@@ -771,8 +772,10 @@ int imc_handle_list(struct sip_msg* msg, imc_cmd_t *cmd,
771772
imc_member_p member = 0;
772773
imc_member_p imp = 0;
773774
str room_name;
774-
str body;
775+
str body = {0, 0};
775776
char *p;
777+
int marker_len;
778+
int entry_len;
776779

777780
/* the user wants to leave the room */
778781
room_name = cmd->param[0].s?cmd->param[0]:dst->user;
@@ -793,7 +796,43 @@ int imc_handle_list(struct sip_msg* msg, imc_cmd_t *cmd,
793796
src->user.len, src->user.s, room_name.len, room_name.s);
794797
goto error;
795798
}
796-
p = imc_body_buf;
799+
800+
body.len = sizeof("Members:\n") - 1;
801+
imp = room->members;
802+
while(imp)
803+
{
804+
if((imp->flags&IMC_MEMBER_INVITED)||(imp->flags&IMC_MEMBER_DELETED)
805+
|| (imp->flags&IMC_MEMBER_SKIP))
806+
{
807+
imp = imp->next;
808+
continue;
809+
}
810+
811+
marker_len = ((imp->flags & IMC_MEMBER_OWNER) ||
812+
(imp->flags & IMC_MEMBER_ADMIN)) ? 1 : 0;
813+
if(imp->uri.len > INT_MAX - marker_len - 1)
814+
{
815+
LM_ERR("member uri too large [%d]\n", imp->uri.len);
816+
goto error;
817+
}
818+
entry_len = marker_len + imp->uri.len + 1;
819+
if(entry_len > INT_MAX - 1 - body.len)
820+
{
821+
LM_ERR("member list too large\n");
822+
goto error;
823+
}
824+
body.len += entry_len;
825+
imp = imp->next;
826+
}
827+
828+
body.s = pkg_malloc(body.len + 1);
829+
if(body.s == NULL)
830+
{
831+
LM_ERR("no more pkg memory\n");
832+
goto error;
833+
}
834+
835+
p = body.s;
797836
memcpy(p, "Members:\n", 9);
798837
p+=9;
799838
imp = room->members;
@@ -810,24 +849,25 @@ int imc_handle_list(struct sip_msg* msg, imc_cmd_t *cmd,
810849
*p++ = '*';
811850
else if(imp->flags & IMC_MEMBER_ADMIN)
812851
*p++ = '~';
813-
strncpy(p, imp->uri.s, imp->uri.len);
852+
memcpy(p, imp->uri.s, imp->uri.len);
814853
p += imp->uri.len;
815854
*p++ = '\n';
816855
imp = imp->next;
817856
}
818857

819-
imc_release_room(room);
820-
821858
/* write over last '\n' */
822859
*(--p) = 0;
823-
body.s = imc_body_buf;
824860
body.len = p-body.s;
825861
LM_DBG("members = [%.*s]\n", body.len, body.s);
826862
imc_send_message(&room->uri, &member->uri, &imc_hdr_ctype, &body);
827863

864+
pkg_free(body.s);
865+
imc_release_room(room);
828866

829867
return 0;
830868
error:
869+
if(body.s)
870+
pkg_free(body.s);
831871
if(room!=NULL)
832872
imc_release_room(room);
833873
return -1;
@@ -1236,4 +1276,3 @@ void imc_inv_callback( struct cell *t, int type, struct tmcb_params *ps)
12361276
shm_free(*ps->param);
12371277
return;
12381278
}
1239-

0 commit comments

Comments
 (0)