From de10a7754b7cfe4f5956ff575113211f222f960b Mon Sep 17 00:00:00 2001
From: Andreas Schneider <asn@cryptomilk.org>
Date: Mon, 2 Feb 2015 16:59:48 +0100
Subject: buffer: buffer: Improve argument checking of in ssh_buffer_pack()

Signed-off-by: Andreas Schneider <asn@cryptomilk.org>
Reviewed-by: Aris Adamantiadis <aris@0xbadc0de.be>
---
 include/libssh/buffer.h | 12 +++++++++---
 src/buffer.c            | 33 ++++++++++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/include/libssh/buffer.h b/include/libssh/buffer.h
index 1cc8196c..826d0b7c 100644
--- a/include/libssh/buffer.h
+++ b/include/libssh/buffer.h
@@ -64,9 +64,15 @@ int _ssh_buffer_pack(struct ssh_buffer_struct *buffer,
 #define ssh_buffer_pack(buffer, format, ...) \
     _ssh_buffer_pack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END)
 
-int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, va_list ap);
-int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer, const char *format, ...);
-#define ssh_buffer_unpack(buffer, format, ...) _ssh_buffer_unpack((buffer),(format), __VA_ARGS__, SSH_BUFFER_PACK_END)
+int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
+                         const char *format, int argc,
+                         va_list ap);
+int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer,
+                       const char *format,
+                       int argc,
+                       ...);
+#define ssh_buffer_unpack(buffer, format, ...) \
+    _ssh_buffer_unpack((buffer), (format), __VA_NARG__(__VA_ARGS__), __VA_ARGS__, SSH_BUFFER_PACK_END)
 
 int buffer_prepend_data(ssh_buffer buffer, const void *data, uint32_t len);
 int buffer_add_buffer(ssh_buffer buffer, ssh_buffer source);
diff --git a/src/buffer.c b/src/buffer.c
index 6bf268a4..983bc789 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -839,7 +839,11 @@ int _ssh_buffer_pack(struct ssh_buffer_struct *buffer,
  *                      SSH_ERROR on error
  * @see ssh_buffer_get_format() for format list values.
  */
-int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, va_list ap){
+int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer,
+                         const char *format,
+                         int argc,
+                         va_list ap)
+{
     int rc = SSH_ERROR;
     const char *p, *last;
     union {
@@ -854,11 +858,17 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, v
     size_t len, rlen;
     uint32_t u32len;
     va_list ap_copy;
+    int count;
 
     /* copy the argument list in case a rollback is needed */
     va_copy(ap_copy, ap);
 
-    for (p = format; *p != '\0'; p++) {
+    for (p = format, count = 0; *p != '\0'; p++, count++) {
+        /* Invalid number of arguments passed */
+        if (count > argc) {
+            return SSH_ERROR;
+        }
+
         switch (*p) {
         case 'b':
             o.byte = va_arg(ap, uint8_t *);
@@ -919,7 +929,10 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, v
             break;
         case 'P':
             len = va_arg(ap, size_t);
+
             o.data = va_arg(ap, void **);
+            count++;
+
             *o.data = malloc(len);
             if(*o.data == NULL){
                 rc = SSH_ERROR;
@@ -942,6 +955,11 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, v
             break;
         }
     }
+
+    if (argc != count) {
+        rc = SSH_ERROR;
+    }
+
     if (rc != SSH_ERROR){
         /* verify that the last hidden argument is correct */
         uint32_t canary = va_arg(ap, uint32_t);
@@ -949,6 +967,7 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, v
             rc = SSH_ERROR;
         }
     }
+
     if (rc != SSH_OK){
         /* Reset the format string and erase everything that was allocated */
         last = p;
@@ -1002,12 +1021,16 @@ int ssh_buffer_unpack_va(struct ssh_buffer_struct *buffer, const char *format, v
  * @warning             when using 'P' with a constant size (e.g. 8), do not
  *                      forget to cast to (size_t).
  */
-int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer, const char *format, ...){
+int _ssh_buffer_unpack(struct ssh_buffer_struct *buffer,
+                       const char *format,
+                       int argc,
+                       ...)
+{
     va_list ap;
     int rc;
 
-    va_start(ap, format);
-    rc = ssh_buffer_unpack_va(buffer, format, ap);
+    va_start(ap, argc);
+    rc = ssh_buffer_unpack_va(buffer, format, argc, ap);
     va_end(ap);
     return rc;
 }
-- 
cgit v1.2.3