From 729b3da96dba9dea52a30f3d1d2046299ab1e0f3 Mon Sep 17 00:00:00 2001 From: Sylvain Rochet Date: Fri, 16 Oct 2020 21:16:05 +0200 Subject: [PATCH] PPP: remove casts from unsigned (strlen return value) to signed when checking auth In theory, if provided username or password is over 0x80000000 byte long (err...), casts to signed integer of strlen() return values is going to return negative values breaking lengths checks. Fix it by only using unsigned integer or size_t (guaranteed to be unsigned) comparisons. --- src/include/netif/ppp/ppp_impl.h | 2 +- src/netif/ppp/auth.c | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/include/netif/ppp/ppp_impl.h b/src/include/netif/ppp/ppp_impl.h index 8d4d834d..218630e7 100644 --- a/src/include/netif/ppp/ppp_impl.h +++ b/src/include/netif/ppp/ppp_impl.h @@ -559,7 +559,7 @@ void start_networks(ppp_pcb *pcb); /* start all the network control protos */ void continue_networks(ppp_pcb *pcb); /* start network [ip, etc] control protos */ #if PPP_AUTH_SUPPORT #if PPP_SERVER -int auth_check_passwd(ppp_pcb *pcb, char *auser, int userlen, char *apasswd, int passwdlen, const char **msg, int *msglen); +int auth_check_passwd(ppp_pcb *pcb, char *auser, unsigned int userlen, char *apasswd, unsigned int passwdlen, const char **msg, int *msglen); /* check the user name and passwd against configuration */ void auth_peer_fail(ppp_pcb *pcb, int protocol); /* peer failed to authenticate itself */ diff --git a/src/netif/ppp/auth.c b/src/netif/ppp/auth.c index 7a74292c..b21ce807 100644 --- a/src/netif/ppp/auth.c +++ b/src/netif/ppp/auth.c @@ -1003,13 +1003,13 @@ void continue_networks(ppp_pcb *pcb) { * 1: Authentication succeeded. * In either case, msg points to an appropriate message and msglen to the message len. */ -int auth_check_passwd(ppp_pcb *pcb, char *auser, int userlen, char *apasswd, int passwdlen, const char **msg, int *msglen) { - int secretuserlen; - int secretpasswdlen; +int auth_check_passwd(ppp_pcb *pcb, char *auser, unsigned int userlen, char *apasswd, unsigned int passwdlen, const char **msg, int *msglen) { + size_t secretuserlen; + size_t secretpasswdlen; if (pcb->settings.user && pcb->settings.passwd) { - secretuserlen = (int)strlen(pcb->settings.user); - secretpasswdlen = (int)strlen(pcb->settings.passwd); + secretuserlen = strlen(pcb->settings.user); + secretpasswdlen = strlen(pcb->settings.passwd); if (secretuserlen == userlen && secretpasswdlen == passwdlen && !memcmp(auser, pcb->settings.user, userlen) @@ -1902,7 +1902,7 @@ have_srp_secret(client, server, need_ip, lacks_ipp) * (We could be either client or server). */ int get_secret(ppp_pcb *pcb, const char *client, const char *server, char *secret, int *secret_len, int am_server) { - int len; + size_t len; LWIP_UNUSED_ARG(server); LWIP_UNUSED_ARG(am_server); @@ -1910,7 +1910,7 @@ int get_secret(ppp_pcb *pcb, const char *client, const char *server, char *secre return 0; } - len = (int)strlen(pcb->settings.passwd); + len = strlen(pcb->settings.passwd); if (len > MAXSECRETLEN) { ppp_error(("Secret for %s on %s is too long", client, server)); len = MAXSECRETLEN; @@ -1922,7 +1922,8 @@ int get_secret(ppp_pcb *pcb, const char *client, const char *server, char *secre #if 0 /* UNUSED */ FILE *f; - int ret, len; + int ret; + size_t len; char *filename; struct wordlist *addrs, *opts; char secbuf[MAXWORDLEN];