psktool: Fix hex-encoding logic of username

The previous code didn't modify the pointer to the realloc'ed region
nor check overflow before calling realloc.

Spotted by Anderson Sasaki in:
<https://gitlab.com/gnutls/gnutls/-/merge_requests/1345#note_439063374>.

Signed-off-by: Daiki Ueno <ueno@gnu.org>

diff --git a/src/psk.c b/src/psk.c
index b79401cf1685061664fe30f0a594db2ce2ee750c..4b8a62957aac67542dacdc48b4354669b5541a83 100644 (file)
--- a/src/psk.c
+++ b/src/psk.c
@@ -58,6 +58,7 @@ int main(int argc, char **argv)
 #include <minmax.h>
 #include "close-stream.h"
 #include "getpass.h"
+#include "xsize.h"
 
 static int write_key(const char *username,
                     const unsigned char *key, size_t key_size,
@@ -217,6 +218,7 @@ write_key(const char *username, const unsigned char *key, size_t key_size,
        /* encode username if it contains special characters */
        if (strcspn(username, ":\n") != strlen(username)) {
                char *new_data;
+               size_t new_size;
 
                tmp.data = (void *)username;
                tmp.size = strlen(username);
@@ -229,16 +231,21 @@ write_key(const char *username, const unsigned char *key, size_t key_size,
                }
 
                /* prepend '#' */
-               new_data = gnutls_realloc(_username.data, _username.size + 2);
+               new_size = xsum(_username.size, 2);
+               if (size_overflow_p(new_size)) {
+                       ret = -1;
+                       goto out;
+               }
+               new_data = gnutls_realloc(_username.data, new_size);
                if (!new_data) {
                        ret = -1;
                        goto out;
                }
-               memmove(_username.data + 1, _username.data, _username.size);
+               memmove(new_data + 1, new_data, _username.size);
                new_data[0] = '#';
-               new_data[_username.size] = '\0';
+               new_data[_username.size + 1] = '\0';
                _username.data = (void *)new_data;
-               _username.size += 1;
+               _username.size = new_size - 1;
        } else {
                _username.data = (void *)strdup(username);
                _username.size = strlen(username);

diff --git a/tests/psktool.sh b/tests/psktool.sh
index 9e81d0171849c1cb3060a4bd9a378beacecdb5f9..9d0e081296111ceb470eb76950b0dc2301243427 100755 (executable)
--- a/tests/psktool.sh
+++ b/tests/psktool.sh
@@ -41,7 +41,7 @@ fi
 echo "Checking PSK tool basic operations"
 
 # echo create a user and check whether a key is available
-"${PSKTOOL}" -p ${TMPFILE} -u test
+${VALGRIND} "${PSKTOOL}" -p ${TMPFILE} -u test
 if test $? != 0;then
        echo "password generation failed..."
        exit 1
@@ -63,7 +63,7 @@ fi
 
 # Create second user and check whether both exist
 
-"${PSKTOOL}" -p ${TMPFILE} -u user2
+${VALGRIND} "${PSKTOOL}" -p ${TMPFILE} -u user2
 if test $? != 0;then
        echo "password generation failed..."
        exit 1
@@ -81,6 +81,34 @@ if test $? != 0;then
        exit 1
 fi
 
+# Create third user with a special character in username
+
+${VALGRIND} "${PSKTOOL}" -p ${TMPFILE} -u user:3
+if test $? != 0;then
+       echo "password generation failed..."
+       exit 1
+fi
+
+grep '#757365723a33:' ${TMPFILE} >/dev/null 2>&1
+if test $? != 0;then
+       echo "could not find third generated user..."
+       exit 1
+fi
+
+# Modify the third user password
+
+${VALGRIND} "${PSKTOOL}" -p ${TMPFILE} -u user:3
+if test $? != 0;then
+       echo "password generation failed..."
+       exit 1
+fi
+
+matches=`grep '#757365723a33:' ${TMPFILE} 2>/dev/null | wc -l`
+if test $matches != 1;then
+       echo "duplicate entry for third generated user..."
+       exit 1
+fi
+
 rm -f $TMPFILE
 
 exit 0