On Thu, Apr 21, 2016 at 8:51 AM, Jens <openvpn-de...@neuhalfen.name> wrote:
> Passing very long usernames/passwords for pam authentication could possibly
> lead to a stack based buffer overrun in the auth-pam plugin.
>
> Adds a dependency to C99 (includes stdbool.h)
>
> Signed-off-by: Jens Neuhalfen <j...@neuhalfen.name>
> ---
> src/plugins/auth-pam/auth-pam.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/src/plugins/auth-pam/auth-pam.c b/src/plugins/auth-pam/auth-pam.c
> index 95692ab..159fd07 100644
> --- a/src/plugins/auth-pam/auth-pam.c
> +++ b/src/plugins/auth-pam/auth-pam.c
> @@ -39,6 +39,7 @@
> #include <stdio.h>
> #include <string.h>
> #include <ctype.h>
> +#include <stdbool.h>
> #include <unistd.h>
> #include <stdlib.h>
> #include <sys/types.h>
> @@ -119,17 +120,37 @@ static void pam_server (int fd, const char *service,
> int verb, const struct name
> * a pointer to the NEW string. Does not modify the input strings. Will
> not enter an
> * infinite loop with clever 'searchfor' and 'replacewith' strings.
> * Daniel Johnson - progman2...@usa.net / djohn...@progman.us
> + *
> + * Retuns NULL when
> + * - any parameter is NULL
> + * - the worst-case result is to large ( >= SIZE_MAX)
> */
> static char *
> searchandreplace(const char *tosearch, const char *searchfor, const char
> *replacewith)
> {
> + if (!tosearch || !searchfor || !replacewith) return NULL;
> +
> + size_t tosearchlen = strlen(tosearch);
> + size_t replacewithlen = strlen(replacewith);
> + size_t templen = tosearchlen * replacewithlen;
> +
> + if (tosearchlen == 0 || strlen(searchfor) == 0 || replacewithlen == 0) {
> + return NULL;
> + }
> +
> + bool is_potential_integer_overflow = (templen == SIZE_MAX) || (templen /
> tosearchlen != replacewithlen);
> +
> + if (is_potential_integer_overflow) {
> + return NULL;
> + }
> +
> + // state: all parameters are valid
> +
> const char *searching=tosearch;
> char *scratch;
> - char temp[strlen(tosearch)*10];
> - temp[0]=0;
>
> - if (!tosearch || !searchfor || !replacewith) return 0;
> - if (!strlen(tosearch) || !strlen(searchfor) || !strlen(replacewith))
> return 0;
> + char temp[templen+1];
> + temp[0]=0;
>
> scratch = strstr(searching,searchfor);
> if (!scratch) return strdup(tosearch);
Thanks for following up.
ACK
-Steffan
