Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix compression overflow bug #2632

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix compression overflow bug #2632

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a5b6d2e
compression: add logging for overflow condition
john08burke Sep 22, 2021
ba89136
compression: fix overflow when hdr_field contains sanitized body
john08burke Sep 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 17 additions & 26 deletions modules/compression/compression.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
#define WORD(p) (*(p + 0) + (*(p + 1) << 8))
#define DWORD(p) (*(p+0) + (*(p+1) << 8) + (*(p+2) << 16) + (*(p+3) << 24))

#define LOWER_CASE(p) (*(p) & 0x20)
#define BUFLEN 4096

#define COMPACT_FORMS "cfiklmstvx"
Expand Down Expand Up @@ -844,31 +843,19 @@ static int mc_compact_cb(char** buf_p, mc_whitelist_p wh_list, int type, int* ol
i = HDR_OTHER_T;
again:
if (hdr_mask[i]) {
/* Compact form name so the header have
to be built */
if (LOWER_CASE(hdr_mask[i]->name.s)) {
/* Copy the name of the header */
wrap_copy_and_update(&new_buf.s,
hdr_mask[i]->name.s,
hdr_mask[i]->name.len, &new_buf.len);

/* Copy the ': ' delimiter*/
wrap_copy_and_update(&new_buf.s, DELIM,
DELIM_LEN, &new_buf.len);
/* Copy the first field of the header*/
wrap_copy_and_update(&new_buf.s,
hdr_mask[i]->body.s,
hdr_mask[i]->body.len, &new_buf.len);
/* Normal form header so it can be copied in one step */
} else {
wrap_copy_and_update(
&new_buf.s,
hdr_mask[i]->name.s,
/* Possible siblings. No CRLF yet */
hdr_mask[i]->len - CRLF_LEN,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @john08burke , I do not have too much expertise over this module, but something tells me that this is not the right place for the fix. How the len of the hdr is calculated here looks ok to me. But what I suspect is that the msg_total_len is badly computed. Do you still have a core file with this crash? or can you reproduce it ? Maybe we can identify where the mismatch between msg_total_len calculation and the writing occurs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bogdan-iancu, I don't have the core file but if I remember correctly it was reproducible in the lab. Let me try to get one later today or tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bogdan-iancu I was able to get the core file. I had to cherry-pick @a5b6d2e2b2f3e9eb72830410d3aaf1819326fe2a and put an abort to get it to dump.

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007faa2e691535 in __GI_abort () at abort.c:79
#2  0x00007faa224fbcaf in mc_compact_cb (buf_p=0x7ffd888875c0, wh_list=0x7faa2af0f5e8, type=1, olen=0x7ffd888875bc) at compression.c:956
#3  0x00007faa224f9b1d in wrap_tm_func (t=0x7faa22a05e80, type=2, p=0x7ffd88887660) at compression.c:307
#4  0x00007faa224f9927 in wrap_tm_compact (t=0x7faa22a05e80, type=8192, p=0x7ffd88887660) at compression.c:278
#5  0x00007faa225d3f6f in run_trans_callbacks (type=8192, trans=0x7faa22a05e80, req=0x7faa2af0c230, rpl=0x0, code=0) at t_hooks.c:209
#6  0x00007faa225e4be7 in t_forward_nonack (t=0x7faa22a05e80, p_msg=0x7faa2af0c230, proxy=0x0, reset_bcounter=1, locked=1) at t_fwd.c:835
#7  0x00007faa225c6a4c in w_t_relay (p_msg=0x7faa2af0c230, flags=0x0, proxy=0x0) at tm.c:1236
#8  0x000055824cb8caf1 in do_action (a=0x7faa2a694800, msg=0x7faa2af0c230) at action.c:974
#9  0x000055824cb89377 in run_action_list (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:158
#10 0x000055824cb89231 in run_actions (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:123
#11 0x000055824cb896c0 in run_top_route (a=0x7faa2a6912f0, msg=0x7faa2af0c230) at action.c:241
#12 0x000055824cb98ddb in receive_msg (
    buf=0x55824cebfe00 <buf> "INVITE sip:2000@192.168.20.148:5060 SIP/2.0\r\nVia: SIP/2.0/UDP 192.168.20.190:5060;branch=teGidaYiiVzxlowwCFqaPPYbLJCWghYxusYp\r\nFrom: <sip:1000@192.168.20.190:5060>;tag=RLCwhxURwvjF\r\nTo: <sip:2000@192."..., len=738, rcv_info=0x7ffd88887b90, existing_context=0x0, msg_flags=0) at receive.c:213
#13 0x000055824cce025f in udp_read_req (si=0x7faa2a68eeb8, bytes_read=0x7ffd88887c58) at net/proto_udp/proto_udp.c:186
#14 0x000055824ccc2185 in handle_io (fm=0x7faa2a6ba590, idx=0, event_type=1) at net/net_udp.c:278
#15 0x000055824ccc0909 in io_wait_loop_epoll (h=0x55824cea9fc0 <_worker_io>, t=1, repeat=0) at net/../io_wait_loop.h:305
#16 0x000055824ccc33fb in udp_start_processes (chd_rank=0x55824cea9e7c <chd_rank>, startup_done=0x0) at net/net_udp.c:503
#17 0x000055824cbf9670 in main_loop () at main.c:802
#18 0x000055824cbfcc9d in main (argc=15, argv=0x7ffd88887f38) at main.c:1491

&new_buf.len
);
}
/* Copy the name of the header */
wrap_copy_and_update(&new_buf.s,
hdr_mask[i]->name.s,
hdr_mask[i]->name.len, &new_buf.len);

/* Copy the ': ' delimiter*/
wrap_copy_and_update(&new_buf.s, DELIM,
DELIM_LEN, &new_buf.len);

/* Copy the first field of the header*/
wrap_copy_and_update(&new_buf.s,
hdr_mask[i]->body.s,
hdr_mask[i]->body.len, &new_buf.len);

/* Copy the rest of the header fields(siblings)
if they exist */
Expand Down Expand Up @@ -950,6 +937,10 @@ static int mc_compact_cb(char** buf_p, mc_whitelist_p wh_list, int type, int* ol
memcpy(*buf_p, new_buf.s, new_buf.len);
*olen = new_buf.len;

if (new_buf.len > msg_total_len)
LM_BUG("buffer overflow: "\
"calculated=%d, actual=%d\n", msg_total_len, new_buf.len);

/* Free the vector */
pkg_free(hdr_mask);

Expand Down