Skip to content

Commit

Permalink
app_rpt.c: Fix issue 469 - truncated link list in RPT_LINK
Browse files Browse the repository at this point in the history
                           Add some comments around the size issues
                           Add TODO to sscanf buffer protection in the future
  • Loading branch information
mkmer committed Jan 28, 2025
1 parent e4dee90 commit e03f764
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 11 deletions.
36 changes: 25 additions & 11 deletions apps/app_rpt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1660,16 +1660,24 @@ static inline void init_text_frame(struct ast_frame *wf)
}

static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *str)
{
char tmp[512], tmp1[512], cmd[300] = "", dest[300], src[30], c;
{
/* These temporary var sizes are a bit tricky:
* tmp is a copy of a string that can be MAXLINKLIST (or larger)
* when we recieve an 'L' it copies tmp into l->linklist which is
* MAXLINKLIST long.
*/
char tmp[MAXLINKLIST], tmp1[512], cmd[300] = "", dest[300], src[30], c;
int i, seq, res, ts, rest;
struct ast_frame wf;

init_text_frame(&wf);
wf.datalen = strlen(str) + 1;
wf.src = "handle_link_data";
/* put string in our buffer */
ast_copy_string(tmp, str, sizeof(tmp) - 1);
if (strlen(str) > sizeof(tmp)) {
ast_log(LOG_WARNING, "Link text message data has exceeded tmp buffer size");
}
ast_copy_string(tmp, str, sizeof(tmp));

ast_debug(5, "Received text over link: '%s'\n", str);

Expand Down Expand Up @@ -1703,15 +1711,15 @@ static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *s
}
if (tmp[0] == 'L') {
rpt_mutex_lock(&myrpt->lock);
strcpy(mylink->linklist, tmp + 2);
strcpy(mylink->linklist, tmp + 2); /* MAXLINKLIST - 2 is placed in mylink->linklist*/
time(&mylink->linklistreceived);
rpt_mutex_unlock(&myrpt->lock);
ast_debug(7, "@@@@ node %s recieved node list %s from node %s\n", myrpt->name, tmp, mylink->name);
return;
}
if (tmp[0] == 'M') {
rest = 0;
if (sscanf(tmp, "%s %s %s %n", cmd, src, dest, &rest) < 3) {
if (sscanf(tmp, "%s %s %s %n", cmd, src, dest, &rest) < 3) { /*TODO: We should limit to sizeof(cmd, src, dest)*/
ast_log(LOG_WARNING, "Unable to parse message string %s\n", str);
return;
}
Expand Down Expand Up @@ -1739,7 +1747,7 @@ static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *s
return;
}
if (tmp[0] == 'T') {
if (sscanf(tmp, "%s %s %s", cmd, src, dest) != 3) {
if (sscanf(tmp, "%s %s %s", cmd, src, dest) != 3) { /*TODO: We should limit to sizeof(cmd, src, dest)*/
ast_log(LOG_WARNING, "Unable to parse telem string %s\n", str);
return;
}
Expand Down Expand Up @@ -1771,7 +1779,7 @@ static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *s
}

if (tmp[0] == 'C') {
if (sscanf(tmp, "%s %s %s %s", cmd, src, tmp1, dest) != 4) {
if (sscanf(tmp, "%s %s %s %s", cmd, src, tmp1, dest) != 4) { /*TODO: We should limit to sizeof(cmd, src, dest, tmp1)*/
ast_log(LOG_WARNING, "Unable to parse ctcss string %s\n", str);
return;
}
Expand All @@ -1790,7 +1798,7 @@ static void handle_link_data(struct rpt *myrpt, struct rpt_link *mylink, char *s
}

if (tmp[0] == 'K') {
if (sscanf(tmp, "%s %s %s %d %d", cmd, dest, src, &seq, &ts) != 5) {
if (sscanf(tmp, "%s %s %s %d %d", cmd, dest, src, &seq, &ts) != 5) { /*TODO: We should limit to sizeof(cmd, src, dest)*/
ast_log(LOG_WARNING, "Unable to parse keying string %s\n", str);
return;
}
Expand Down Expand Up @@ -2139,10 +2147,16 @@ static int handle_remote_dtmf_digit(struct rpt *myrpt, char c, char *keyed, int

static int handle_remote_data(struct rpt *myrpt, char *str)
{
char tmp[300], cmd[300], dest[300], src[300], c;
/* These temproary var sizes are a bit tricky:
* tmp is a copy of a string that can be MAXLINKLIST (or larger)
*/
char tmp[MAXLINKLIST], cmd[300], dest[300], src[300], c;
int seq, res;

/* put string in our buffer */
if (strlen(str) > sizeof(tmp)) {
ast_log(LOG_WARNING, "Remove link text message data has exceeded tmp buffer size");
}
ast_copy_string(tmp, str, sizeof(tmp));
if (!strcmp(tmp, DISCSTR))
return 0;
Expand All @@ -2167,7 +2181,7 @@ static int handle_remote_data(struct rpt *myrpt, char *str)

#ifndef DO_NOT_NOTIFY_MDC1200_ON_REMOTE_BASES
if (tmp[0] == 'I') {
if (sscanf(tmp, "%s %s %s", cmd, src, dest) != 3) {
if (sscanf(tmp, "%s %s %s", cmd, src, dest) != 3) { /*TODO: these should be limited to sizeof(cmd,dest,src)*/
ast_log(LOG_WARNING, "Unable to parse ident string %s\n", str);
return 0;
}
Expand All @@ -2177,7 +2191,7 @@ static int handle_remote_data(struct rpt *myrpt, char *str)
#endif
if (tmp[0] == 'L')
return 0;
if (sscanf(tmp, "%s %s %s %d %c", cmd, dest, src, &seq, &c) != 5) {
if (sscanf(tmp, "%s %s %s %d %c", cmd, dest, src, &seq, &c) != 5) { /*TODO: these should be limited to sizeof(cmd,dest,src)*/
ast_log(LOG_WARNING, "Unable to parse link string %s\n", str);
return 0;
}
Expand Down
7 changes: 7 additions & 0 deletions apps/app_rpt/rpt_link.c
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,13 @@ void __mklinklist(struct rpt *myrpt, struct rpt_link *mylink, char *buf, int fla
if (flag) {
snprintf(buf + spos, MAXLINKLIST - spos, "%s%c%c", l->name, mode, (l->lastrx1) ? 'K' : 'U');
} else {

/* l->linklist and buff have a size of MAXLINKLIST. We will have a problem with large networks where this exceeds MAXLINKLIST
* worst case(ish) The message looks like this "<network size>, 6 digit node + T + ,"
* With MAXLINKLIST = 5120, 5120 - 4 / 8 = 638 nodes. We can have smaller node values but generally we
* will have problems on networks > 638 nodes as is.
*/

/* add nodes into buffer */
if (l->linklist[0]) {
snprintf(buf + spos, MAXLINKLIST - spos, "%c%s,%s", mode, l->name, l->linklist);
Expand Down

0 comments on commit e03f764

Please sign in to comment.