diff mbox

[9/9] libnsdb: do not read beyond end of malloc'd buffer; do NUL-terminate

Message ID 1322755106-8171-10-git-send-email-jim@meyering.net
State Superseded
Headers show

Commit Message

Jim Meyering Dec. 1, 2011, 3:58 p.m. UTC 
From: Jim Meyering <meyering@redhat.com>

nsdb_sanitize_annotation backslash-escapes its input string into
malloc'd storage.  Worst case is that is has to escape every input
byte.  That means allocating 2*N bytes for the actual data, and
one more for the trailing NUL byte.  That last byte was not allocated.
At first, I thought there must be a buffer overrun.  But no, because
there was a second problem: the trailing NUL byte was never written.
If it worked it all, it worked by accident, because the unused part
of the malloc'd buffer happened to be zero-filled.
* src/libnsdb/annotation.c (nsdb_sanitize_annotation): Correct
two problems:
1) allocated too little memory, by one byte
2) did not NUL-terminate the output buffer
Callers thinking they have a NUL-terminated string would segfault if
there happened to be many non-NUL bytes after the sanitized string.
Found by inspection.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 src/libnsdb/annotation.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
diff mbox

Patch

diff --git a/src/libnsdb/annotation.c b/src/libnsdb/annotation.c
index 2447275..f54500f 100644
--- a/src/libnsdb/annotation.c
+++ b/src/libnsdb/annotation.c
@@ -114,7 +114,7 @@  nsdb_sanitize_annotation(const char *in, char **out)

 	/* Assume worst case: every input character must be escaped */
 	len = strlen(in);
-	result = malloc(len * 2);
+	result = malloc(len * 2 + 1);
 	if (result == NULL) {
 		xlog(D_GENERAL, "%s: Failed to allocate output buffer",
 			__func__);
@@ -129,6 +129,9 @@  nsdb_sanitize_annotation(const char *in, char **out)
 		result[j++] = in[i];
 	}

+	/* NUL-terminate */
+	result[j] = 0;
+
 	*out = result;
 	xlog(D_CALL, "%s: out_len = %zu, out = \"%s\"",
 		__func__, j, result);