From bf668044ae5e4c6adfad786a4b3ac5d55bac74ca Mon Sep 17 00:00:00 2001 From: vaclav Date: Thu, 1 Dec 2022 11:36:09 +0100 Subject: [PATCH 1/6] Issue 233: Improve robustness of command-line parameters; under IMPROVE_CMDLINE_ROBUSTNESS --- apps/encoder.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++- lib_com/options.h | 2 ++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/apps/encoder.c b/apps/encoder.c index 0bad34878b..b53a97fa93 100644 --- a/apps/encoder.c +++ b/apps/encoder.c @@ -1273,8 +1273,21 @@ static bool parseCmdlIVAS_enc( arg->inputFormat = IVAS_ENC_INPUT_ISM; i++; +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + if ( i < argc - 4 ) +#else if ( i < argc - 5 ) +#endif { +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + if ( !is_digits_only( argv[i] ) ) + { + fprintf( stderr, "Error: Number of ISM channels must be an integer number!\n\n" ); + usage_enc(); + return false; + } +#endif + if ( sscanf( argv[i], "%d", &tmp ) > 0 ) { i++; @@ -1286,6 +1299,14 @@ static bool parseCmdlIVAS_enc( usage_enc(); return false; } +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + else if ( tmp > IVAS_MAX_NUM_OBJECTS ) + { + fprintf( stderr, "Error: Too high number of ISM channels specified!\n\n" ); + usage_enc(); + return false; + } +#endif else { arg->inputFormatConfig.ism.numObjects = (int16_t) tmp; @@ -1317,7 +1338,11 @@ static bool parseCmdlIVAS_enc( } else { +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + fprintf( stderr, "Error: not enough metadata arguments specified!\n\n" ); +#else fprintf( stderr, "Error: not enough arguments\n\n" ); +#endif usage_enc(); return false; } @@ -1359,7 +1384,19 @@ static bool parseCmdlIVAS_enc( arg->inputFormatConfig.sba.order = IVAS_ENC_SBA_HOA3; break; default: + +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + if ( is_number( argv[i - 1] ) ) + { + fprintf( stderr, "Error: Wrong SBA order specified!\n\n" ); + } + else + { + fprintf( stderr, "Error: SBA order specified must be a number!\n\n" ); + } +#else fprintf( stderr, "Error: Wrong SBA order specified!\n\n" ); +#endif usage_enc(); return false; } @@ -1371,6 +1408,15 @@ static bool parseCmdlIVAS_enc( if ( i < argc - 4 ) { +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + if ( !is_digits_only( argv[i] ) ) + { + fprintf( stderr, "Error: Number of MASA channels must be an integer number!\n\n" ); + usage_enc(); + return false; + } +#endif + if ( sscanf( argv[i], "%d", &tmp ) > 0 ) { i++; @@ -1385,7 +1431,11 @@ static bool parseCmdlIVAS_enc( arg->inputFormatConfig.masaVariant = IVAS_ENC_MASA_2CH; break; default: +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + fprintf( stderr, "Error: MASA channels must be 1 or 2.\n\n" ); +#else fprintf( stderr, "Error: MASA channels must for the moment be 1 or 2.\n\n" ); +#endif usage_enc(); return false; } @@ -1410,7 +1460,6 @@ static bool parseCmdlIVAS_enc( if ( i < argc - 4 ) { - if ( strcmp( to_upper( argv[i] ), "5_1" ) == 0 ) { arg->inputFormatConfig.mcLayout = IVAS_ENC_MC_5_1; diff --git a/lib_com/options.h b/lib_com/options.h index 388f61bd95..a31a0f3ac0 100644 --- a/lib_com/options.h +++ b/lib_com/options.h @@ -175,6 +175,8 @@ #define FIX_GET_DELAY_RETURN /* Issue 223: change return data type in function get_delay() */ #define NTT_REDUC_COMP_POC /* NTT Contribution 10: Complexity reduction of phase spectrum in stereo downmix*/ #define FIX_ISM_DECODER_PRINTOUT /* Issue 229: fix ISM decoder printout */ +#define IMPROVE_CMDLINE_ROBUSTNESS /* Issue 233: Improve robustness of command-line parameters */ + /* ################## End DEVELOPMENT switches ######################### */ -- GitLab From 6c0c3da989a66a5f41a3d085338849cb8f230e06 Mon Sep 17 00:00:00 2001 From: Archit Tamarapu Date: Mon, 5 Dec 2022 12:40:55 +0100 Subject: [PATCH 2/6] update is_number() function (cherry picked from commit f55e16cfc318b16a51bd3454d32a0fcc5c18c2af) --- lib_util/cmdl_tools.c | 47 +++++++++++++++++++++++++++++++++++++++---- lib_util/cmdl_tools.h | 4 ++-- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib_util/cmdl_tools.c b/lib_util/cmdl_tools.c index d0d2d0d374..593409456a 100644 --- a/lib_util/cmdl_tools.c +++ b/lib_util/cmdl_tools.c @@ -62,7 +62,7 @@ char *to_upper( char *str ) * Check if a string contains only digits. *---------------------------------------------------------------------*/ -bool is_digits_only( char *str ) +bool is_digits_only( const char *str ) { int16_t i; @@ -86,19 +86,58 @@ bool is_digits_only( char *str ) * Check if a string is a number. *---------------------------------------------------------------------*/ -bool is_number( char *str ) +bool is_number( const char *str ) { int16_t i; + int16_t decimal_separator_count; + int16_t numeric_len; i = 0; + decimal_separator_count = 0; + numeric_len = 0; + + /* Check for null string or sign character */ + if ( str[i] == '\0' ) + { + return false; + } + else if ( str[i] == '+' || str[i] == '-' ) + { + i++; + } + + /* Ensure rest of string is numeric and only one decimal separator is present */ while ( str[i] != 0 ) { - if ( ( str[i] < '0' || str[i] > '9' ) && str[i] != '.' && str[i] != '-' && str[i] != '\n' && str[i] != '\r' ) + if ( str[i] < '0' || str[i] > '9' ) { - return false; + if ( str[i] == '.' ) + { + if ( decimal_separator_count > 1 ) + { + return false; + } + else + { + decimal_separator_count++; + } + } + else if ( str[i] != '\r' && str[i] != '\n' ) + { + return false; + } + } + else + { + numeric_len++; } i++; } + if ( numeric_len == 0 ) + { + return false; + } + return true; } diff --git a/lib_util/cmdl_tools.h b/lib_util/cmdl_tools.h index 829ec985b0..52b6e69453 100644 --- a/lib_util/cmdl_tools.h +++ b/lib_util/cmdl_tools.h @@ -36,9 +36,9 @@ #include #include -bool is_digits_only( char *str ); +bool is_digits_only( const char *str ); -bool is_number( char *str ); +bool is_number( const char *str ); char *to_upper( char *str ); -- GitLab From 7ba34e4b1452a7c213134dee1a7eee8469248ba9 Mon Sep 17 00:00:00 2001 From: Archit Tamarapu Date: Mon, 5 Dec 2022 12:46:34 +0100 Subject: [PATCH 3/6] minor modification for readability (cherry picked from commit c598d237314b7050745eff5927f1d209e34709c0) --- lib_util/cmdl_tools.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib_util/cmdl_tools.c b/lib_util/cmdl_tools.c index 593409456a..d36112f8a3 100644 --- a/lib_util/cmdl_tools.c +++ b/lib_util/cmdl_tools.c @@ -88,12 +88,12 @@ bool is_digits_only( const char *str ) bool is_number( const char *str ) { + bool decimal_separator_found; int16_t i; - int16_t decimal_separator_count; int16_t numeric_len; + decimal_separator_found = false; i = 0; - decimal_separator_count = 0; numeric_len = 0; /* Check for null string or sign character */ @@ -113,13 +113,13 @@ bool is_number( const char *str ) { if ( str[i] == '.' ) { - if ( decimal_separator_count > 1 ) + if ( decimal_separator_found ) { return false; } else { - decimal_separator_count++; + decimal_separator_found = true; } } else if ( str[i] != '\r' && str[i] != '\n' ) -- GitLab From 7531cd4abd580486bc7574f5fa552aa9c755ac34 Mon Sep 17 00:00:00 2001 From: Shanush Prema Thasarathan Date: Tue, 6 Dec 2022 08:09:43 +1100 Subject: [PATCH 4/6] Move check of is_number earlier in the logic to fail early --- apps/encoder.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/apps/encoder.c b/apps/encoder.c index b53a97fa93..476d3a1936 100644 --- a/apps/encoder.c +++ b/apps/encoder.c @@ -1354,12 +1354,20 @@ static bool parseCmdlIVAS_enc( arg->inputFormat = IVAS_ENC_INPUT_SBA; /* SBA configuration */ - if ( i < argc - 4 ) + if ( i < argc - 4 +#ifdef IMPROVE_CMDLINE_ROBUSTNESS + && is_number(argv[i]) && sscanf( argv[i], "%d", &tmp ) > 0 +#endif + ) { +#ifndef IMPROVE_CMDLINE_ROBUSTNESS if ( sscanf( argv[i], "%d", &tmp ) > 0 ) { +#endif i++; +#ifndef IMPROVE_CMDLINE_ROBUSTNESS } +#endif } else { @@ -1384,19 +1392,7 @@ static bool parseCmdlIVAS_enc( arg->inputFormatConfig.sba.order = IVAS_ENC_SBA_HOA3; break; default: - -#ifdef IMPROVE_CMDLINE_ROBUSTNESS - if ( is_number( argv[i - 1] ) ) - { - fprintf( stderr, "Error: Wrong SBA order specified!\n\n" ); - } - else - { - fprintf( stderr, "Error: SBA order specified must be a number!\n\n" ); - } -#else fprintf( stderr, "Error: Wrong SBA order specified!\n\n" ); -#endif usage_enc(); return false; } -- GitLab From 36f5a1fc018e3208d619963242031cfc787fda64 Mon Sep 17 00:00:00 2001 From: Shanush Prema Thasarathan Date: Tue, 6 Dec 2022 08:18:31 +1100 Subject: [PATCH 5/6] Clang format fixes --- apps/encoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/encoder.c b/apps/encoder.c index 476d3a1936..a91532f9e6 100644 --- a/apps/encoder.c +++ b/apps/encoder.c @@ -1354,9 +1354,9 @@ static bool parseCmdlIVAS_enc( arg->inputFormat = IVAS_ENC_INPUT_SBA; /* SBA configuration */ - if ( i < argc - 4 + if ( i < argc - 4 #ifdef IMPROVE_CMDLINE_ROBUSTNESS - && is_number(argv[i]) && sscanf( argv[i], "%d", &tmp ) > 0 + && is_number( argv[i] ) && sscanf( argv[i], "%d", &tmp ) > 0 #endif ) { -- GitLab From dd136dee6bf216769a4833b7eb316415206ca1e9 Mon Sep 17 00:00:00 2001 From: Shanush Prema Thasarathan Date: Wed, 7 Dec 2022 08:40:45 +1100 Subject: [PATCH 6/6] Change error message to indicate the nubmer is expected --- apps/encoder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/encoder.c b/apps/encoder.c index a91532f9e6..005794d939 100644 --- a/apps/encoder.c +++ b/apps/encoder.c @@ -1372,7 +1372,7 @@ static bool parseCmdlIVAS_enc( else { tmp = -1; /* this is to avoid a compilation warning */ - fprintf( stderr, "Error: SBA order not specified!\n\n" ); + fprintf( stderr, "Error: SBA order must be specified, expecting a number!\n\n" ); usage_enc(); return false; } -- GitLab