[Asterisk-code-review] Iostreams: Correct off-by-one error. (asterisk[master])

[Joshua Colp]

Joshua Colp has submitted this change and it was merged. ( https://gerrit.asterisk.org/4556 )

Change subject: Iostreams: Correct off-by-one error.
......................................................................


Iostreams: Correct off-by-one error.

ast_iostream_printf() attempts first to use a fixed-size buffer to
perform its printf-like operation. If the fixed-size buffer is too
small, then a heap allocation is used instead. The heap allocation in
this case was exactly the length of the string to print. The issue here
is that the ensuing call to vsnprintf() will print a NULL byte in the
final space of the string. This meant that the final character was being
chopped off the string and replaced with a NULL byte. For HTTP in
particular, this caused problems because HTTP publishes the expected
Contact-Length. This meant HTTP was publishing a length one character
larger than what was actually present in the message.

This patch corrects the issue by adding one to the allocation length.

ASTERISK-26629
Reported by Joshua Colp

Change-Id: Ib3c5f41e96833d0415cf000656ac368168add639
---
M main/iostream.c
1 file changed, 8 insertions(+), 5 deletions(-)

Approvals:
  Richard Mudgett: Looks good to me, but someone else must approve
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, approved



diff --git a/main/iostream.c b/main/iostream.c
index a20a048..22cd598 100644
--- a/main/iostream.c
+++ b/main/iostream.c
@@ -404,7 +404,7 @@
 
 ssize_t ast_iostream_printf(struct ast_iostream *stream, const void *fmt, ...)
 {
-	char sbuf[256], *buf = sbuf;
+	char sbuf[512], *buf = sbuf;
 	int len, len2, ret = -1;
 	va_list va;
 
@@ -412,15 +412,18 @@
 	len = vsnprintf(buf, sizeof(sbuf), fmt, va);
 	va_end(va);
 
-	if (len > sizeof(sbuf)) {
-		buf = ast_malloc(len);
+	if (len > sizeof(sbuf) - 1) {
+		/* Add one to the string length to accommodate the NULL byte */
+		size_t buf_len = len + 1;
+
+		buf = ast_malloc(buf_len);
 		if (!buf) {
 			return -1;
 		}
 		va_start(va, fmt);
-		len2 = vsnprintf(buf, len, fmt, va);
+		len2 = vsnprintf(buf, buf_len, fmt, va);
 		va_end(va);
-		if (len2 > len) {
+		if (len2 != len) {
 			goto error;
 		}
 	}

-- 
To view, visit https://gerrit.asterisk.org/4556
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib3c5f41e96833d0415cf000656ac368168add639
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>