From 838dfb8058df62af44905e1fa3936d989844effc Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 25 Sep 2015 22:04:58 +0200 Subject: [PATCH] Provide safer string copy than strncpy As per @boxerab suggestion in commit b9ca882749597e4943e609daba1d3c27694fe2ff --- src/bin/common/opj_string.h | 72 ++++++++++++++++++++++++++++++++++++ src/bin/jp2/CMakeLists.txt | 1 + src/bin/jp2/opj_compress.c | 32 +++++++++++----- src/bin/jp2/opj_decompress.c | 49 +++++++++++++++--------- src/bin/jp2/opj_dump.c | 20 +++++++--- 5 files changed, 141 insertions(+), 33 deletions(-) create mode 100644 src/bin/common/opj_string.h diff --git a/src/bin/common/opj_string.h b/src/bin/common/opj_string.h new file mode 100644 index 00000000..8829926a --- /dev/null +++ b/src/bin/common/opj_string.h @@ -0,0 +1,72 @@ +/* + * The copyright in this software is being made available under the 2-clauses + * BSD License, included below. This software may be subject to other third + * party and contributor rights, including patent rights, and no such rights + * are granted under this license. + * + * Copyright (c) 2015, Matthieu Darbois + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS `AS IS' + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef OPJ_STRING_H +#define OPJ_STRING_H + +#include +#include + +/* strnlen is not standard, strlen_s is C11... */ +/* keep in mind there still is a buffer read overflow possible */ +static size_t opj_strnlen_s(const char *src, size_t max_len) +{ + size_t len; + + if (src == NULL) { + return 0U; + } + for (len = 0U; (*src != '\0') && (len < max_len); src++, len++); + return len; +} + +/* should be equivalent to C11 function except for the handler */ +/* keep in mind there still is a buffer read overflow possible */ +static int opj_strcpy_s(char* dst, size_t dst_size, const char* src) +{ + size_t src_len = 0U; + if ((dst == NULL) || (dst_size == 0U)) { + return EINVAL; + } + if (src == NULL) { + dst[0] = '\0'; + return EINVAL; + } + src_len = opj_strnlen_s(src, dst_size); + if (src_len >= dst_size) { + return ERANGE; + } + memcpy(dst, src, src_len); + dst[src_len] = '\0'; + return 0; +} + +#endif /* OPJ_STRING_H */ diff --git a/src/bin/jp2/CMakeLists.txt b/src/bin/jp2/CMakeLists.txt index 7e2476df..d583c2e6 100644 --- a/src/bin/jp2/CMakeLists.txt +++ b/src/bin/jp2/CMakeLists.txt @@ -11,6 +11,7 @@ set(common_SRCS ${OPENJPEG_SOURCE_DIR}/src/bin/common/color.h ${OPENJPEG_SOURCE_DIR}/src/bin/common/opj_getopt.c ${OPENJPEG_SOURCE_DIR}/src/bin/common/opj_getopt.h + ${OPENJPEG_SOURCE_DIR}/src/bin/common/opj_string.h ) if(OPJ_HAVE_LIBTIFF) diff --git a/src/bin/jp2/opj_compress.c b/src/bin/jp2/opj_compress.c index e5f98532..9d690a56 100644 --- a/src/bin/jp2/opj_compress.c +++ b/src/bin/jp2/opj_compress.c @@ -69,6 +69,7 @@ #include "index.h" #include "format_defs.h" +#include "opj_string.h" typedef struct dircnt{ /** Buffer for holding images read from Directory*/ @@ -456,8 +457,10 @@ static char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_c if (parameters->decod_format == -1) return 1; sprintf(infilename,"%s/%s",img_fol->imgdirpath,image_filename); - strncpy(parameters->infile, infilename, sizeof(infilename) - 1U); - + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infilename) != 0) { + return 1; + } + /*Set output file*/ strcpy(temp_ofname,get_file_name(image_filename)); while((temp_p = strtok(NULL,".")) != NULL){ @@ -466,7 +469,9 @@ static char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_c } if(img_fol->set_out_format==1){ sprintf(outfilename,"%s/%s.%s",img_fol->imgdirpath,temp_ofname,img_fol->out_format); - strncpy(parameters->outfile, outfilename, sizeof(outfilename) - 1U); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), outfilename) != 0) { + return 1; + } } return 0; } @@ -474,7 +479,7 @@ static char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_c /* ------------------------------------------------------------------------------------ */ static int parse_cmdline_encoder(int argc, char **argv, opj_cparameters_t *parameters, - img_fol_t *img_fol, raw_cparameters_t *raw_cp, char *indexfilename) { + img_fol_t *img_fol, raw_cparameters_t *raw_cp, char *indexfilename, size_t indexfilename_size) { OPJ_UINT32 i, j; int totlen, c; opj_option_t long_option[]={ @@ -528,7 +533,9 @@ static int parse_cmdline_encoder(int argc, char **argv, opj_cparameters_t *param infile); return 1; } - strncpy(parameters->infile, infile, sizeof(parameters->infile)-1); + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infile) != 0) { + return 1; + } } break; @@ -546,7 +553,9 @@ static int parse_cmdline_encoder(int argc, char **argv, opj_cparameters_t *param fprintf(stderr, "Unknown output format image %s [only *.j2k, *.j2c or *.jp2]!! \n", outfile); return 1; } - strncpy(parameters->outfile, outfile, sizeof(parameters->outfile)-1); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), outfile) != 0) { + return 1; + } } break; @@ -812,8 +821,9 @@ static int parse_cmdline_encoder(int argc, char **argv, opj_cparameters_t *param case 'x': /* creation of index file */ { - char *index = opj_optarg; - strncpy(indexfilename, index, OPJ_PATH_LEN); + if (opj_strcpy_s(indexfilename, indexfilename_size, opj_optarg) != 0) { + return 1; + } /* FIXME ADE INDEX >> */ fprintf(stderr, "[WARNING] Index file generation is currently broken.\n" @@ -1133,7 +1143,9 @@ static int parse_cmdline_encoder(int argc, char **argv, opj_cparameters_t *param /* we need to enable indexing */ if (!indexfilename || !*indexfilename) { - strncpy(indexfilename, JPWL_PRIVATEINDEX_NAME, OPJ_PATH_LEN); + if (opj_strcpy_s(indexfilename, indexfilename_size, JPWL_PRIVATEINDEX_NAME) != 0) { + return 1; + } } /* search for different protection methods */ @@ -1622,7 +1634,7 @@ int main(int argc, char **argv) { /* parse input and get user encoding parameters */ parameters.tcp_mct = (char) 255; /* This will be set later according to the input image or the provided option */ - if(parse_cmdline_encoder(argc, argv, ¶meters,&img_fol, &raw_cp, indexfilename) == 1) { + if(parse_cmdline_encoder(argc, argv, ¶meters,&img_fol, &raw_cp, indexfilename, sizeof(indexfilename)) == 1) { return 1; } diff --git a/src/bin/jp2/opj_decompress.c b/src/bin/jp2/opj_decompress.c index c27dc201..3db36ebe 100644 --- a/src/bin/jp2/opj_decompress.c +++ b/src/bin/jp2/opj_decompress.c @@ -75,6 +75,7 @@ #include "color.h" #include "format_defs.h" +#include "opj_string.h" typedef struct dircnt{ /** Buffer for holding images read from Directory*/ @@ -121,6 +122,8 @@ typedef struct opj_decompress_params int decod_format; /** output file format 0: PGX, 1: PxM, 2: BMP */ int cod_format; + /** index file name */ + char indexfilename[OPJ_PATH_LEN]; /** Decoding area left boundary */ OPJ_UINT32 DA_x0; @@ -157,7 +160,7 @@ int get_file_format(const char *filename); char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_decompress_parameters *parameters); static int infile_format(const char *fname); -int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *parameters,img_fol_t *img_fol, char *indexfilename); +int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *parameters,img_fol_t *img_fol); int parse_DA_values( char* inArg, unsigned int *DA_x0, unsigned int *DA_y0, unsigned int *DA_x1, unsigned int *DA_y1); static opj_image_t* convert_gray_to_rgb(opj_image_t* original); @@ -429,7 +432,9 @@ char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_decompre parameters->decod_format = infile_format(infilename); if (parameters->decod_format == -1) return 1; - strncpy(parameters->infile, infilename, sizeof(infilename)); + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infilename) != 0) { + return 1; + } /*Set output file*/ strcpy(temp_ofname,strtok(image_filename,".")); @@ -439,7 +444,9 @@ char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_decompre } if(img_fol->set_out_format==1){ sprintf(outfilename,"%s/%s.%s",img_fol->imgdirpath,temp_ofname,img_fol->out_format); - strncpy(parameters->outfile, outfilename, sizeof(outfilename)); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), outfilename) != 0) { + return 1; + } } return 0; } @@ -505,7 +512,7 @@ static int infile_format(const char *fname) * Parse the command line */ /* -------------------------------------------------------------------------- */ -int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *parameters,img_fol_t *img_fol, char *indexfilename) { +int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *parameters,img_fol_t *img_fol) { /* parse the command line */ int totlen, c; opj_option_t long_option[]={ @@ -561,7 +568,10 @@ int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *para infile); return 1; } - strncpy(parameters->infile, infile, sizeof(parameters->infile)-1); + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infile) != 0) { + fprintf(stderr, "[ERROR] Path is too long\n"); + return 1; + } } break; @@ -592,7 +602,10 @@ int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *para fprintf(stderr, "Unknown output format image %s [only *.pnm, *.pgm, *.ppm, *.pgx, *.bmp, *.tif, *.raw or *.tga]!! \n", outfile); return 1; } - strncpy(parameters->outfile, outfile, sizeof(parameters->outfile)-1); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), outfile) != 0) { + fprintf(stderr, "[ERROR] Path is too long\n"); + return 1; + } } break; @@ -676,11 +689,14 @@ int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *para case 'd': /* Input decode ROI */ { - int size_optarg = (int)strlen(opj_optarg) + 1; - char *ROI_values = (char*) malloc((size_t)size_optarg); + size_t size_optarg = (size_t)strlen(opj_optarg) + 1U; + char *ROI_values = (char*) malloc(size_optarg); + if (ROI_values == NULL) { + fprintf(stderr, "[ERROR] Couldn't allocate memory\n"); + return 1; + } ROI_values[0] = '\0'; - strncpy(ROI_values, opj_optarg, strlen(opj_optarg)); - ROI_values[strlen(opj_optarg)] = '\0'; + memcpy(ROI_values, opj_optarg, size_optarg); /*printf("ROI_values = %s [%d / %d]\n", ROI_values, strlen(ROI_values), size_optarg ); */ parse_DA_values( ROI_values, ¶meters->DA_x0, ¶meters->DA_y0, ¶meters->DA_x1, ¶meters->DA_y1); @@ -701,8 +717,10 @@ int parse_cmdline_decoder(int argc, char **argv, opj_decompress_parameters *para case 'x': /* Creation of index file */ { - char *index = opj_optarg; - strncpy(indexfilename, index, OPJ_PATH_LEN); + if (opj_strcpy_s(parameters->indexfilename, sizeof(parameters->indexfilename), opj_optarg) != 0) { + fprintf(stderr, "[ERROR] Path is too long\n"); + return 1; + } } break; @@ -1167,8 +1185,6 @@ int main(int argc, char **argv) opj_codec_t* l_codec = NULL; /* Handle to a decompressor */ opj_codestream_index_t* cstr_index = NULL; - char indexfilename[OPJ_PATH_LEN]; /* index file name */ - OPJ_INT32 num_images, imageno; img_fol_t img_fol; dircnt_t *dirptr = NULL; @@ -1179,14 +1195,11 @@ int main(int argc, char **argv) /* set decoding parameters to default values */ set_default_parameters(¶meters); - /* FIXME Initialize indexfilename and img_fol */ - *indexfilename = 0; - /* Initialize img_fol */ memset(&img_fol,0,sizeof(img_fol_t)); /* parse input and get user encoding parameters */ - if(parse_cmdline_decoder(argc, argv, ¶meters,&img_fol, indexfilename) == 1) { + if(parse_cmdline_decoder(argc, argv, ¶meters,&img_fol) == 1) { destroy_parameters(¶meters); return EXIT_FAILURE; } diff --git a/src/bin/jp2/opj_dump.c b/src/bin/jp2/opj_dump.c index 4005f1c2..562fd5a5 100644 --- a/src/bin/jp2/opj_dump.c +++ b/src/bin/jp2/opj_dump.c @@ -57,6 +57,7 @@ #include "index.h" #include "format_defs.h" +#include "opj_string.h" typedef struct dircnt{ /** Buffer for holding images read from Directory*/ @@ -196,7 +197,9 @@ static char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_d if (parameters->decod_format == -1) return 1; sprintf(infilename,"%s/%s",img_fol->imgdirpath,image_filename); - strncpy(parameters->infile, infilename, sizeof(infilename)); + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infilename) != 0) { + return 1; + } /*Set output file*/ strcpy(temp_ofname,strtok(image_filename,".")); @@ -206,7 +209,9 @@ static char get_next_file(int imageno,dircnt_t *dirptr,img_fol_t *img_fol, opj_d } if(img_fol->set_out_format==1){ sprintf(outfilename,"%s/%s.%s",img_fol->imgdirpath,temp_ofname,img_fol->out_format); - strncpy(parameters->outfile, outfilename, sizeof(outfilename)); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), outfilename) != 0) { + return 1; + } } return 0; } @@ -303,7 +308,10 @@ static int parse_cmdline_decoder(int argc, char **argv, opj_dparameters_t *param infile); return 1; } - strncpy(parameters->infile, infile, sizeof(parameters->infile)-1); + if (opj_strcpy_s(parameters->infile, sizeof(parameters->infile), infile) != 0) { + fprintf(stderr, "[ERROR] Path is too long\n"); + return 1; + } } break; @@ -311,8 +319,10 @@ static int parse_cmdline_decoder(int argc, char **argv, opj_dparameters_t *param case 'o': /* output file */ { - char *outfile = opj_optarg; - strncpy(parameters->outfile, outfile, sizeof(parameters->outfile)-1); + if (opj_strcpy_s(parameters->outfile, sizeof(parameters->outfile), opj_optarg) != 0) { + fprintf(stderr, "[ERROR] Path is too long\n"); + return 1; + } } break;