Skip to content

Commit

Permalink
usrloc User-Agent filtering: Fix memory corruption
Browse files Browse the repository at this point in the history
The "ua_re_check" macro added by commit e5cb980 is broken in several ways:
    * unsafe read operation on shared memory
    * unsafe write operation on shared memory
    * incorrectly handled error case, without restoring the backup byte

This patch corrects the above issues by extending the user_agent buffer.

(cherry picked from commit 58a944c)

Includes fix commits:
    - 03398fb
    - e16abb4

(cherry picked from commit a9bdf7b)
  • Loading branch information
liviuchircu committed Oct 3, 2016
1 parent 82fd3ae commit 7f44695
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 11 deletions.
3 changes: 0 additions & 3 deletions modules/registrar/lookup.c
Expand Up @@ -53,12 +53,9 @@

#define ua_re_check(return) \
if (flags & REG_LOOKUP_UAFILTER_FLAG) { \
tmp = *(ptr->user_agent.s+ptr->user_agent.len); \
*(ptr->user_agent.s+ptr->user_agent.len) = '\0'; \
if (regexec(&ua_re, ptr->user_agent.s, 1, &ua_match, 0)) { \
return; \
} \
*(ptr->user_agent.s+ptr->user_agent.len) = tmp; \
}

/*! \brief
Expand Down
21 changes: 13 additions & 8 deletions modules/usrloc/ucontact.c
Expand Up @@ -114,7 +114,9 @@ ucontact_t* new_ucontact(str* _dom, str* _aor, str* _contact, ucontact_info_t* _

if (shm_str_dup( &c->c, _contact) < 0) goto mem_error;
if (shm_str_dup( &c->callid, _ci->callid) < 0) goto mem_error;
if (shm_str_dup( &c->user_agent, _ci->user_agent) < 0) goto mem_error;

/* an additional null byte may be needed by "regexec" later on */
if (shm_nt_str_dup( &c->user_agent, _ci->user_agent) < 0) goto mem_error;

if (_ci->received.s && _ci->received.len) {
if (shm_str_dup( &c->received, &_ci->received) < 0) goto mem_error;
Expand Down Expand Up @@ -246,10 +248,11 @@ void print_ucontact(FILE* _f, ucontact_t* _c)
*/
int mem_update_ucontact(ucontact_t* _c, ucontact_info_t* _ci)
{
#define update_str(_old,_new) \
/* "user_agent" must be null-terminated (see e5cb9805b) */
#define update_str(_old,_new,_nt) \
do{\
if ((_old)->len < (_new)->len) { \
ptr = (char*)shm_malloc((_new)->len); \
ptr = shm_malloc((_new)->len + ((_nt) ? 1 : 0)); \
if (ptr == 0) { \
LM_ERR("no more shm memory\n"); \
return -1; \
Expand All @@ -261,6 +264,8 @@ int mem_update_ucontact(ucontact_t* _c, ucontact_info_t* _ci)
memcpy((_old)->s, (_new)->s, (_new)->len);\
}\
(_old)->len = (_new)->len;\
if (_nt) \
(_old)->s[(_old)->len] = '\0'; \
} while(0)

char* ptr;
Expand All @@ -269,28 +274,28 @@ int mem_update_ucontact(ucontact_t* _c, ucontact_info_t* _ci)
* the same Call-ID header field value for registrations sent
* to a particular registrar.', but it is not a 'MUST'. So
* always update the call ID to be safe. */
update_str( &_c->callid, _ci->callid);
update_str( &_c->callid, _ci->callid, 0);

update_str( &_c->user_agent, _ci->user_agent);
update_str( &_c->user_agent, _ci->user_agent, 1);

if (_ci->received.s && _ci->received.len) {
update_str( &_c->received, &_ci->received);
update_str( &_c->received, &_ci->received, 0);
} else {
if (_c->received.s) shm_free(_c->received.s);
_c->received.s = 0;
_c->received.len = 0;
}

if (_ci->path) {
update_str( &_c->path, _ci->path);
update_str( &_c->path, _ci->path, 0);
} else {
if (_c->path.s) shm_free(_c->path.s);
_c->path.s = 0;
_c->path.len = 0;
}

if (_ci->attr && _ci->attr->s && _ci->attr->len) {
update_str( &_c->attr, _ci->attr);
update_str( &_c->attr, _ci->attr, 0);
} else {
if (_c->attr.s) shm_free(_c->attr.s);
_c->attr.s = 0;
Expand Down
18 changes: 18 additions & 0 deletions ut.h
Expand Up @@ -529,6 +529,24 @@ static inline int shm_str_dup(str* dst, const str* src)
return 0;
}

/*
* Make a copy of an str structure using shm_malloc
* + an additional '\0' byte, so you can make use of dst->s
*/
static inline int shm_nt_str_dup(str* dst, const str* src)
{
dst->s = shm_malloc(src->len + 1);
if (!dst->s) {
LM_ERR("no shared memory left\n");
return -1;
}

memcpy(dst->s, src->s, src->len);
dst->len = src->len;
dst->s[dst->len] = '\0';
return 0;
}

/*
* Make a copy of a str structure using pkg_malloc
*/
Expand Down

0 comments on commit 7f44695

Please sign in to comment.