The features_struct.size variable is used to hold a buffer size and it is also passed in as the size parameter to read(). It should be a size_t instead of an int.
A new helper function, features_buffer_remaining(), is created to handle the two places where the remaining bytes in the features buffer are calculated. This patch also changes the size parameter of load_features_dir() to a size_t to match the same parameter of load_features_file() as well as the features_struct.size change described above. Two casts were needed when comparing signed types to unsigned types. These casts are safe because the signed value is checked for "< 0" immediately before the casts. Signed-off-by: Tyler Hicks <[email protected]> --- * Changes since v1: - Subtract fst->buffer from fst->pos and ensure the result is not greater than remaining before subtracting - Move the remaining buffer calculation into a helper function - Use the helper function in features_dir_cb(), which didn't have a sanity check on the value of remaining libraries/libapparmor/src/features.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/libraries/libapparmor/src/features.c b/libraries/libapparmor/src/features.c index 088c4ea..c656c37 100644 --- a/libraries/libapparmor/src/features.c +++ b/libraries/libapparmor/src/features.c @@ -23,6 +23,7 @@ #include <stdio.h> #include <string.h> #include <stdarg.h> +#include <stddef.h> #include <stdlib.h> #include <sys/types.h> #include <sys/stat.h> @@ -42,7 +43,7 @@ struct aa_features { struct features_struct { char *buffer; - int size; + size_t size; char *pos; }; @@ -51,17 +52,30 @@ struct component { size_t len; }; -static int features_snprintf(struct features_struct *fst, const char *fmt, ...) +static int features_buffer_remaining(struct features_struct *fst, + size_t *remaining) { - va_list args; - int i, remaining = fst->size - (fst->pos - fst->buffer); + ptrdiff_t offset = fst->pos - fst->buffer; - if (remaining < 0) { + if (offset < 0 || fst->size < (size_t)offset) { errno = EINVAL; - PERROR("Invalid features buffer offset\n"); + PERROR("Invalid features buffer offset (%td)\n", offset); return -1; } + *remaining = fst->size - offset; + return 0; +} + +static int features_snprintf(struct features_struct *fst, const char *fmt, ...) +{ + va_list args; + int i; + size_t remaining; + + if (features_buffer_remaining(fst, &remaining) == -1) + return -1; + va_start(args, fmt); i = vsnprintf(fst->pos, remaining, fmt, args); va_end(args); @@ -70,7 +84,7 @@ static int features_snprintf(struct features_struct *fst, const char *fmt, ...) errno = EIO; PERROR("Failed to write to features buffer\n"); return -1; - } else if (i >= remaining) { + } else if ((size_t)i >= remaining) { errno = ENOBUFS; PERROR("Feature buffer full."); return -1; @@ -157,7 +171,10 @@ static int features_dir_cb(int dirfd, const char *name, struct stat *st, if (S_ISREG(st->st_mode)) { ssize_t len; - int remaining = fst->size - (fst->pos - fst->buffer); + size_t remaining; + + if (features_buffer_remaining(fst, &remaining) == -1) + return -1; len = load_features_file(dirfd, name, fst->pos, remaining); if (len < 0) @@ -176,7 +193,7 @@ static int features_dir_cb(int dirfd, const char *name, struct stat *st, } static ssize_t load_features_dir(int dirfd, const char *path, - char *buffer, int size) + char *buffer, size_t size) { struct features_struct fst = { buffer, size, buffer }; -- 2.9.3 -- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
