From 6027aa2247f2c9bc2c3dabb126db62dfc5358b3c Mon Sep 17 00:00:00 2001
From: Martin Lambers <martin.lambers@ruhr-uni-bochum.de>
Date: Thu, 17 Oct 2024 14:42:53 +0200
Subject: [PATCH] Validate verification code input before sending it

This makes sure that the verification code only contains characters a-z A-Z
0-9, and is of limited length.

Otherwise it is possible for a user to cause arbitrary data to be sent to the
server, including invalid JSON. This could be a problem if the JSON parser on
the receiving end has bugs.

For example, code input 'A"}BBBBBBBB...' would lead to JSON data
'{"session_id":"SESSIONID","pin":"A"}BBBBBBBB..."}' to be sent to the server.
---
 src/pam_weblogin.c |  7 +++++++
 src/pam_weblogin.h |  2 ++
 src/tty.c          | 18 ++++++++++++++++++
 src/tty.h          |  2 ++
 4 files changed, 29 insertions(+)

diff --git a/src/pam_weblogin.c b/src/pam_weblogin.c
index 44cc6dd..3711c1f 100644
--- a/src/pam_weblogin.c
+++ b/src/pam_weblogin.c
@@ -157,6 +157,13 @@ PAM_EXTERN int pam_sm_authenticate(pam_handle_t *pamh, UNUSED int flags, int arg
 		 ++retry)
 	{
 		char *code = tty_input(pamh, PROMPT_CODE, PAM_PROMPT_ECHO_OFF);
+		if (!input_is_safe(code, MAX_INPUT_LENGTH))
+		{
+			tty_output(pamh, "invalid code");
+			log_message(LOG_ERR, "invalid code");
+			free(code);
+			continue;
+		}
 
 		/* Prepare URL... */
 		url = str_printf("%s/%s", cfg->url, API_CHECK_CODE_PATH);
diff --git a/src/pam_weblogin.h b/src/pam_weblogin.h
index 2edf1cc..7506059 100644
--- a/src/pam_weblogin.h
+++ b/src/pam_weblogin.h
@@ -16,6 +16,8 @@
 #define PROMPT_GROUP "\nSelect group: "
 #define PROMPT_WRONG_NUMBER "Wrong number"
 
+#define MAX_INPUT_LENGTH 256
+
 #ifndef GIT_COMMIT
 #define GIT_COMMIT 0000
 #endif
diff --git a/src/tty.c b/src/tty.c
index de1db1f..b4f1af7 100644
--- a/src/tty.c
+++ b/src/tty.c
@@ -77,6 +77,24 @@ char *tty_input(pam_handle_t *pamh, const char *text, int echo_code)
 	return ret;
 }
 
+int input_is_safe(const char *input, size_t max_length)
+{
+    for (size_t i = 0; input[i]; i++)
+    {
+        if (i > max_length
+                /* Don't use isalnum() here because it is locale-dependent,
+                 * and don't use isalnum_l() because it is not portable.
+                 * Instead, hardcode a check for ASCII a-z A-Z 0-9. */
+                || !(  (input[i] >= 'a' && input[i] <= 'z')
+                    || (input[i] >= 'A' && input[i] <= 'Z')
+                    || (input[i] >= '0' && input[i] <= '9')))
+        {
+            return 0;
+        }
+    }
+    return 1;
+}
+
 void tty_output(pam_handle_t *pamh, const char *text)
 {
 	/* note: on MacOS, pam_message.msg is a non-const char*, so we need to copy it */
diff --git a/src/tty.h b/src/tty.h
index de13ced..d3c1ad6 100644
--- a/src/tty.h
+++ b/src/tty.h
@@ -2,6 +2,7 @@
 #define TTY_H
 
 #include <syslog.h>
+#include <stddef.h>
 
 #include <security/pam_appl.h>
 #include <security/pam_modules.h>
@@ -10,6 +11,7 @@ void log_message(int priority, const char *fmt, ...)
 	__attribute__ ((format (printf, 2, 3)));
 
 char *tty_input(pam_handle_t *pamh, const char *text, int echo_code);
+int input_is_safe(const char *input, size_t max_length);
 void tty_output(pam_handle_t *pamh, const char *text);
 
 #endif /* TTY_H */