[Asterisk-code-review] bridge_channel: Ensure text messages are zero terminated (asterisk[17])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/14773 )

Change subject: bridge_channel: Ensure text messages are zero terminated
......................................................................

bridge_channel: Ensure text messages are zero terminated

T.140 data in RTP is not zero terminated, so when we are queuing a text
frame on a bridge we need to ensure that we are passing a zero
terminated string.

ASTERISK-28974 #close

Change-Id: Ic10057387ce30b2094613ea67e3ae8c5c431dda3
---
M include/asterisk/frame.h
M main/bridge_channel.c
2 files changed, 40 insertions(+), 2 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/include/asterisk/frame.h b/include/asterisk/frame.h
index 8892765..c5267b1 100644
--- a/include/asterisk/frame.h
+++ b/include/asterisk/frame.h
@@ -107,7 +107,10 @@
 	AST_FRAME_NULL,
 	/*! Inter Asterisk Exchange private frame type */
 	AST_FRAME_IAX,
-	/*! Text messages */
+	/*! Text messages. The character data may not be zero-terminated, so
+	 * care should be taken when passing it to functions that expect a
+	 * zero-terminated string. The frame's datalen member should be used
+	 * as it indicates the actual number of bytes available. */
 	AST_FRAME_TEXT,
 	/*! Image Frames */
 	AST_FRAME_IMAGE,
diff --git a/main/bridge_channel.c b/main/bridge_channel.c
index 251dea7..ade98fd 100644
--- a/main/bridge_channel.c
+++ b/main/bridge_channel.c
@@ -2377,6 +2377,41 @@
 
 /*!
  * \internal
+ * \brief Ensure text data is zero terminated before sending
+ *
+ * \param chan Channel to send text to
+ * \param f The frame containing the text data to send
+ *
+ * \return Nothing
+ */
+static void sendtext_safe(struct ast_channel *chan, const struct ast_frame *f)
+{
+	if (f->datalen) {
+		char *text = f->data.ptr;
+
+		if (text[f->datalen - 1]) {
+			/* Not zero terminated, we need to allocate */
+			text = ast_strndup(text, f->datalen);
+			if (!text) {
+				return;
+			}
+		}
+
+		ast_sendtext(chan, text);
+
+		if (text != f->data.ptr) {
+			/* Only free if we allocated */
+			ast_free(text);
+		}
+	} else {
+		/* Special case if the frame length is zero (although I
+		 * am not sure this is possible?) */
+		ast_sendtext(chan, "");
+	}
+}
+
+/*!
+ * \internal
  * \brief Handle bridge channel write frame to channel.
  * \since 12.0.0
  *
@@ -2449,7 +2484,7 @@
 	case AST_FRAME_TEXT:
 		ast_debug(1, "Sending TEXT frame to '%s': %*.s\n",
 			ast_channel_name(bridge_channel->chan), fr->datalen, (char *)fr->data.ptr);
-		ast_sendtext(bridge_channel->chan, fr->data.ptr);
+		sendtext_safe(bridge_channel->chan, fr);
 		break;
 	case AST_FRAME_TEXT_DATA:
 		msg = (struct ast_msg_data *)fr->data.ptr;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14773
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 17
Gerrit-Change-Id: Ic10057387ce30b2094613ea67e3ae8c5c431dda3
Gerrit-Change-Number: 14773
Gerrit-PatchSet: 4
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200825/48478cd8/attachment-0001.html>