diff options
| author | Holger Hans Peter Freyther <holger@moiji-mobile.com> | 2014-05-22 15:03:10 +0200 |
|---|---|---|
| committer | Holger Hans Peter Freyther <holger@moiji-mobile.com> | 2014-05-22 16:46:11 +0200 |
| commit | 1cc5ff8e19c840545ae746c52995d0768c2d1f70 (patch) | |
| tree | b9288badaf39184d9f015073e1fc8b792ce68de8 | |
| parent | f259bd8359de61e9c8aa92c775b95665c1a143d5 (diff) | |
mgcp: Add proper length checking for line handling
In ae1997248ccb4fba1394267d3051082dfd85448a the handwritten tokenizer
was replaced with strtok_r. As part of this change the structural
checking of MGCP parameters was stopped. This means that a code like
"line + 3" might access beyond the first NUL and be possibly behind
the msgb. Manually add size checking again. Manually jumping to the
error label is not possible anymore as it has been removed. The result
is that invalid lines will be skipped. This is matching the general
approach by the IETF RFCs to be permissive in data being received.
| -rw-r--r-- | openbsc/src/libmgcp/mgcp_protocol.c | 22 | ||||
| -rw-r--r-- | openbsc/tests/mgcp/mgcp_test.c | 1 |
2 files changed, 23 insertions, 0 deletions
diff --git a/openbsc/src/libmgcp/mgcp_protocol.c b/openbsc/src/libmgcp/mgcp_protocol.c index 1480acca3..d4088a166 100644 --- a/openbsc/src/libmgcp/mgcp_protocol.c +++ b/openbsc/src/libmgcp/mgcp_protocol.c @@ -109,6 +109,19 @@ static void delete_transcoder(struct mgcp_endpoint *endp); static int mgcp_analyze_header(struct mgcp_parse_data *parse, char *data); +static int mgcp_check_param(const struct mgcp_endpoint *endp, const char *line) +{ + const size_t line_len = strlen(line); + if (line[0] != '\0' && line_len < 2) { + LOGP(DMGCP, LOGL_ERROR, + "Wrong MGCP option format: '%s' on 0x%x\n", + line, ENDPOINT_NUMBER(endp)); + return 0; + } + + return 1; +} + static uint32_t generate_call_id(struct mgcp_config *cfg) { int i; @@ -750,6 +763,9 @@ static struct msgb *handle_create_con(struct mgcp_parse_data *p) /* parse CallID C: and LocalParameters L: */ for_each_line(line, p->save) { + if (!mgcp_check_param(endp, line)) + continue; + switch (line[0]) { case 'L': local_options = (const char *) line + 3; @@ -901,6 +917,9 @@ static struct msgb *handle_modify_con(struct mgcp_parse_data *p) } for_each_line(line, p->save) { + if (!mgcp_check_param(endp, line)) + continue; + switch (line[0]) { case 'C': { if (verify_call_id(endp, line + 3) != 0) @@ -1028,6 +1047,9 @@ static struct msgb *handle_delete_con(struct mgcp_parse_data *p) } for_each_line(line, p->save) { + if (!mgcp_check_param(endp, line)) + continue; + switch (line[0]) { case 'C': if (verify_call_id(endp, line + 3) != 0) diff --git a/openbsc/tests/mgcp/mgcp_test.c b/openbsc/tests/mgcp/mgcp_test.c index 498a1d8e6..53884833f 100644 --- a/openbsc/tests/mgcp/mgcp_test.c +++ b/openbsc/tests/mgcp/mgcp_test.c @@ -177,6 +177,7 @@ static void test_strline(void) #define CRCX "CRCX 2 1@mgw MGCP 1.0\r\n" \ "M: recvonly\r\n" \ "C: 2\r\n" \ + "X\r\n" \ "L: p:20\r\n" \ "\r\n" \ "v=0\r\n" \ |