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

login
register
mail settings
Submitter Chuck Lever
Date Dec. 13, 2011, 10:51 p.m.
Message ID <20111213225104.15402.20779.stgit@degas.1015granger.net>
Download mbox | patch
Permalink /patch/131201/
State Accepted
Headers show

Comments

Chuck Lever - Dec. 13, 2011, 10:51 p.m.
From: Jim Meyering <jim@meyering.net>

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.

Introduced by commit 0520ee72: "Initial commit," (March 29, 2011).

Signed-off-by: Jim Meyering <meyering@redhat.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---

 src/libnsdb/annotation.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Patch

diff --git a/src/libnsdb/annotation.c b/src/libnsdb/annotation.c
index 2447275..6444122 100644
--- a/src/libnsdb/annotation.c
+++ b/src/libnsdb/annotation.c
@@ -101,7 +101,7 @@ 
  * Check for UTF-8 cleanliness and provide proper escaping
  *
  * @param in NUL-terminated C string containing string to sanitize
- * @param out OUT: dynamically allocated C string containing cleansed string
+ * @param out OUT: NUL-terminated C string containing cleansed value
  * @return a FedFsStatus code
  *
  * Caller must free "out" with free(3)
@@ -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__);
@@ -128,6 +128,7 @@  nsdb_sanitize_annotation(const char *in, char **out)
 
 		result[j++] = in[i];
 	}
+	result[j] = '\0';
 
 	*out = result;
 	xlog(D_CALL, "%s: out_len = %zu, out = \"%s\"",