From 4c7923413ed2f327ebc4875dcde98a04865e80d9 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 19 Jul 2012 22:34:45 -0400 Subject: [PATCH] Implemented server-side bounds checking on inccoming SPA data. Enhanced the libfko decoding routine to include bounds checking on decrypted SPA data. This includes verifying the number of fields within incoming SPA data (colon separated) along with verifying string lengths of each field. --- lib/fko_decode.c | 81 ++++++++++++++++++++++++++++++++++++++------ lib/fko_encryption.c | 3 +- lib/fko_limits.h | 5 +++ 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/lib/fko_decode.c b/lib/fko_decode.c index 08a52c47..abc29759 100644 --- a/lib/fko_decode.c +++ b/lib/fko_decode.c @@ -5,7 +5,7 @@ * * Author: Damien S. Stuart * - * Purpose: Decrypt and decode an FKO SPA message. + * Purpose: Decode an FKO SPA message after decryption. * * Copyright 2009-2010 Damien Stuart (dstuart@dstuart.org) * @@ -34,13 +34,13 @@ #include "base64.h" #include "digest.h" -/* Decrypt the encoded SPA data. +/* Decode the encoded SPA data. */ int fko_decode_spa_data(fko_ctx_t ctx) { - char *tbuf, *ndx; - int t_size; + char *tbuf, *ndx, *tmp; + int t_size, i; /* Check for required data. */ @@ -48,13 +48,21 @@ fko_decode_spa_data(fko_ctx_t ctx) || strlen(ctx->encoded_msg) < MIN_SPA_ENCODED_MSG_SIZE) return(FKO_ERROR_INVALID_DATA); - /* Move the Digest to its place in the context. + /* Make sure there are enough fields in the SPA packet + * delimited with ':' chars */ - ndx = strrchr(ctx->encoded_msg, ':'); /* Find the last : in the data */ - if(ndx == NULL) - return(FKO_ERROR_INVALID_DATA); + ndx = ctx->encoded_msg; + for (i=0; i < MAX_SPA_FIELDS; i++) + { + if ((tmp = strchr(ndx, ':')) == NULL) + break; - ndx++; + ndx = tmp; + ndx++; + } + + if (i < MIN_SPA_FIELDS) + return(FKO_ERROR_INVALID_DATA); t_size = strlen(ndx); @@ -132,7 +140,7 @@ fko_decode_spa_data(fko_ctx_t ctx) /* We give up here if the computed digest does not match the * digest in the message data. */ - if(strcmp(ctx->digest, tbuf)) + if(strncmp(ctx->digest, tbuf, t_size)) { free(tbuf); return(FKO_ERROR_DIGEST_VERIFICATION_FAILED); @@ -168,6 +176,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_USERNAME_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + strlcpy(tbuf, ndx, t_size+1); ctx->username = malloc(t_size+1); /* Yes, more than we need */ @@ -188,6 +202,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_TIMESTAMP_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + strlcpy(tbuf, ndx, t_size+1); ctx->timestamp = (unsigned int)atoi(tbuf); @@ -201,6 +221,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_VERSION_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + ctx->version = malloc(t_size+1); if(ctx->version == NULL) { @@ -219,6 +245,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_MESSAGE_TYPE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + strlcpy(tbuf, ndx, t_size+1); ctx->message_type = (unsigned int)atoi(tbuf); @@ -232,6 +264,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_MESSAGE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + strlcpy(tbuf, ndx, t_size+1); ctx->message = malloc(t_size+1); /* Yes, more than we need */ @@ -257,6 +295,12 @@ fko_decode_spa_data(fko_ctx_t ctx) return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_MESSAGE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + strlcpy(tbuf, ndx, t_size+1); ctx->nat_access = malloc(t_size+1); /* Yes, more than we need */ @@ -274,6 +318,12 @@ fko_decode_spa_data(fko_ctx_t ctx) ndx += t_size + 1; if((t_size = strlen(ndx)) > 0) { + if (t_size > MAX_SPA_MESSAGE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + /* There is data, but what is it? * If the message_type does not have a timeout, assume it is a * server_auth field. @@ -314,6 +364,12 @@ fko_decode_spa_data(fko_ctx_t ctx) { t_size = strcspn(ndx, ":"); + if (t_size > MAX_SPA_MESSAGE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } + /* Looks like we have both, so assume this is the */ strlcpy(tbuf, ndx, t_size+1); @@ -341,6 +397,11 @@ fko_decode_spa_data(fko_ctx_t ctx) free(tbuf); return(FKO_ERROR_INVALID_DATA); } + if (t_size > MAX_SPA_MESSAGE_SIZE) + { + free(tbuf); + return(FKO_ERROR_INVALID_DATA); + } /* Should be a number only. */ diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 4b81a411..bc2a80a4 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -157,7 +157,8 @@ _rijndael_decrypt(fko_ctx_t ctx, const char *dec_key) /* At this point we can check the data to see if we have a good * decryption by ensuring the first field (16-digit random decimal - * value) is valid and is followed by a colon. + * value) is valid and is followed by a colon. Additional checks + * are made in fko_decode_spa_data(). */ ndx = (unsigned char *)ctx->encoded_msg; for(i=0; i