From b016438c08877cb3b24808d260397a8aa8fe49ce Mon Sep 17 00:00:00 2001 From: axel Date: Wed, 21 Jun 2006 10:48:36 +1000 Subject: [PATCH] Update input validation. Always use the magic CHECK macro, which prints an error message including instructions on how to report this problem. darcs-hash:20060621004836-ac50b-a47f296634eda0c469eb39034603015b1ad7ab5c.gz --- builtin.c | 32 ++++++----------------- common.c | 33 +++++++----------------- common.h | 20 +++++++++++++++ complete.c | 42 +++++++++---------------------- env.c | 32 ++++++----------------- exec.c | 8 ++---- expand.c | 11 ++++++++ function.c | 71 ++++++++++++++-------------------------------------- highlight.c | 9 ++----- parse_util.c | 14 ++++++++--- parser.c | 49 ++++++++++-------------------------- tokenizer.c | 42 ++++++++++++++++++------------- 12 files changed, 139 insertions(+), 224 deletions(-) diff --git a/builtin.c b/builtin.c index c8ddab6f2..c092c83d5 100644 --- a/builtin.c +++ b/builtin.c @@ -3015,12 +3015,8 @@ void builtin_destroy() int builtin_exists( wchar_t *cmd ) { - if( !cmd ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( cmd, 0 ); + /* Count is not a builtin, but it's help is handled internally by fish, so it is in the hash_table_t. @@ -3052,12 +3048,9 @@ int builtin_run( wchar_t **argv ) { int (*cmd)(wchar_t **argv)=0; - if( !argv || !argv[0] ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( argv, 1 ); + CHECK( argv[0], 1 ); + cmd = (int (*)(wchar_t **))hash_get( &builtin, argv[0] ); if( argv[1] != 0 && !internal_help(argv[0]) ) @@ -3087,24 +3080,15 @@ int builtin_run( wchar_t **argv ) void builtin_get_names( array_list_t *list ) { - if( !list ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( list, ); + hash_get_keys( &builtin, list ); } const wchar_t *builtin_get_desc( const wchar_t *b ) { - if( !b ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } + CHECK( b, 0 ); - if( !desc ) { int i; diff --git a/common.c b/common.c index bbbd8fca3..095687972 100644 --- a/common.c +++ b/common.c @@ -495,12 +495,8 @@ int contains_str( const wchar_t *a, ... ) va_list va; int res = 0; - if( !a ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( a, 0 ); + va_start( va, a ); while( (arg=va_arg(va, wchar_t *) )!= 0 ) { @@ -546,12 +542,8 @@ void debug( int level, const wchar_t *msg, ... ) string_buffer_t sb; string_buffer_t sb2; - if( !msg ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( msg, ); + if( level > debug_level ) return; @@ -578,12 +570,9 @@ void write_screen( const wchar_t *msg, string_buffer_t *buff ) int tok_width = 0; int screen_width = common_get_width(); - if( !msg || !buff ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( msg, ); + CHECK( buff, ); + if( screen_width ) { start = pos = msg; @@ -799,12 +788,8 @@ wchar_t *unescape( const wchar_t * orig, int unescape_special ) wchar_t prev=0; wchar_t *in; - if( !orig ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( orig, 0 ); + len = wcslen( orig ); in = wcsdup( orig ); diff --git a/common.h b/common.h index c231d6a07..393770692 100644 --- a/common.h +++ b/common.h @@ -67,6 +67,26 @@ extern char *profile; */ extern wchar_t *program_name; +/** + This macro is used to check that an input argument is not null. It + is a bit lika a non-fatal form of assert. Instead of exit-ing on + failiure, the current function is ended at once. The second + parameter is the exit status of the current function on failiure. +*/ +#define CHECK( arg, retval ) \ + if( !(arg) ) \ + { \ + debug( 1, \ + L"function %s called with null value for argument %s. " \ + L"This is a bug. " \ + L"If you can reproduce it, please send a bug report to %s.", \ + __func__, \ + #arg, \ + PACKAGE_BUGREPORT ); \ + return retval; \ + } \ + + /** Take an array_list_t containing wide strings and converts them to a single null-terminated wchar_t **. The array is allocated using diff --git a/complete.c b/complete.c index 71c14d78e..349a353ae 100644 --- a/complete.c +++ b/complete.c @@ -389,12 +389,8 @@ void complete_add( const wchar_t *cmd, complete_entry *c; complete_entry_opt *opt; - if( !cmd ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( cmd, ); + c = complete_find_exact_entry( cmd, cmd_type ); if( c == 0 ) @@ -461,12 +457,8 @@ void complete_remove( const wchar_t *cmd, { complete_entry *e, *eprev=0, *enext=0; - if( !cmd ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( cmd, ); + for( e = first_entry; e; e=enext ) { enext=e->next; @@ -615,12 +607,9 @@ int complete_is_valid_option( const wchar_t *str, void *context; - if( !str || !opt ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( str, 0 ); + CHECK( opt, 0 ); + /* Check some generic things like -- and - options. */ @@ -942,12 +931,8 @@ const wchar_t *complete_get_desc( const wchar_t *filename ) { struct stat buf; - if( !filename ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( filename, 0 ); + if( !get_desc_buff ) { get_desc_buff = sb_halloc( global_context); @@ -1978,12 +1963,9 @@ void complete( const wchar_t *cmd, int cursor_pos = wcslen(cmd ); - if( !cmd || !comp ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( cmd, ); + CHECK( comp, ); + /** If we are completing a variable name or a tilde expansion user name, we do that and return. No need for any other competions. diff --git a/env.c b/env.c index e8f482c76..dd0edf39c 100644 --- a/env.c +++ b/env.c @@ -627,12 +627,8 @@ int env_set( const wchar_t *key, event_t ev; int is_universal = 0; - if( !key ) - { - debug( 0, L"%s called with null input", __func__ ); - return ENV_INVALID; - } - + CHECK( key, ENV_INVALID ); + if( (var_mode & ENV_USER ) && hash_get( &env_read_only, key ) ) { @@ -877,12 +873,8 @@ int env_remove( const wchar_t *key, int var_mode ) env_node_t *first_node; int erased = 0; - if( !key ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( key, 1 ); + if( (var_mode & ENV_USER ) && hash_get( &env_read_only, key ) ) { @@ -941,12 +933,8 @@ wchar_t *env_get( const wchar_t *key ) env_node_t *env = top; wchar_t *item; - if( !key ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( key, 0 ); + if( wcscmp( key, L"history" ) == 0 ) { wchar_t *current; @@ -1045,12 +1033,8 @@ int env_exist( const wchar_t *key, int mode ) env_node_t *env; wchar_t *item=0; - if( !key ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( key, 0 ); + /* Read only variables all exist, and they are all global. A local version can not exist. diff --git a/exec.c b/exec.c index 66f7e1d57..ee0d6b800 100644 --- a/exec.c +++ b/exec.c @@ -652,12 +652,8 @@ void exec( job_t *j ) */ int exec_error=0; - if( !j ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( j, ); + if( no_exec ) return; diff --git a/expand.c b/expand.c index 75f6f964f..255933114 100644 --- a/expand.c +++ b/expand.c @@ -101,6 +101,8 @@ int expand_is_clean( const wchar_t *in ) const wchar_t * str = in; + CHECK( in, 1 ); + /* Test characters that have a special meaning in the first character position */ @@ -161,6 +163,8 @@ wchar_t *expand_escape_variable( const wchar_t *in ) array_list_t l; string_buffer_t buff; + CHECK( in, 0 ); + al_init( &l ); tokenize_variable_array( in, &l ); sb_init( &buff ); @@ -1316,6 +1320,8 @@ static wchar_t * expand_tilde_internal( wchar_t *in ) wchar_t *expand_tilde( wchar_t *in) { + CHECK( in, 0 ); + if( in[0] == L'~' ) { in[0] = HOME_DIRECTORY; @@ -1374,6 +1380,9 @@ int expand_string( void *context, int res = EXPAND_OK; int start_count = al_get_count( end_out ); + CHECK( str, EXPAND_ERROR ); + CHECK( end_out, EXPAND_ERROR ); + if( (!(flags & ACCEPT_INCOMPLETE)) && expand_is_clean( str ) ) { halloc_register( context, str ); @@ -1619,6 +1628,8 @@ wchar_t *expand_one( void *context, wchar_t *string, int flags ) array_list_t l; int res; wchar_t *one; + + CHECK( string, 0 ); if( (!(flags & ACCEPT_INCOMPLETE)) && expand_is_clean( string ) ) { diff --git a/function.c b/function.c index e04366e2b..0fd05ce2c 100644 --- a/function.c +++ b/function.c @@ -176,13 +176,9 @@ void function_add( const wchar_t *name, wchar_t *cmd_end; function_data_t *d; + CHECK( name, ); + CHECK( val, ); - if( !name || !val ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - function_remove( name ); d = malloc( sizeof( function_data_t ) ); @@ -212,12 +208,8 @@ void function_add( const wchar_t *name, int function_exists( const wchar_t *cmd ) { - if( !cmd ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( cmd, 0 ); + if( parser_is_reserved(cmd) ) return 0; @@ -232,12 +224,8 @@ void function_remove( const wchar_t *name ) function_data_t *d; event_t ev; - if( !name ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( name, ); + hash_remove( &function, name, &key, @@ -259,12 +247,8 @@ const wchar_t *function_get_definition( const wchar_t *argv ) { function_data_t *data; - if( !argv ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( argv, 0 ); + load( argv ); data = (function_data_t *)hash_get( &function, argv ); if( data == 0 ) @@ -275,12 +259,9 @@ const wchar_t *function_get_definition( const wchar_t *argv ) const wchar_t *function_get_desc( const wchar_t *argv ) { function_data_t *data; - if( !argv ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } + CHECK( argv, 0 ); + load( argv ); data = (function_data_t *)hash_get( &function, argv ); if( data == 0 ) @@ -292,11 +273,9 @@ const wchar_t *function_get_desc( const wchar_t *argv ) void function_set_desc( const wchar_t *name, const wchar_t *desc ) { function_data_t *data; - if( !name || !desc ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } + + CHECK( name, ); + CHECK( desc, ); load( name ); data = (function_data_t *)hash_get( &function, name ); @@ -355,12 +334,8 @@ static void get_names_internal_all( void *key, void function_get_names( array_list_t *list, int get_hidden ) { - if( !list ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( list, ); + autoload_names( list, get_hidden ); if( get_hidden ) @@ -378,12 +353,8 @@ const wchar_t *function_get_definition_file( const wchar_t *argv ) { function_data_t *data; - if( !argv ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( argv, 0 ); + load( argv ); data = (function_data_t *)hash_get( &function, argv ); if( data == 0 ) @@ -397,12 +368,8 @@ int function_get_definition_offset( const wchar_t *argv ) { function_data_t *data; - if( !argv ) - { - debug( 0, L"%s called with null input", __func__ ); - return -1; - } - + CHECK( argv, -1 ); + load( argv ); data = (function_data_t *)hash_get( &function, argv ); if( data == 0 ) diff --git a/highlight.c b/highlight.c index 86c0623dd..cea56bf3f 100644 --- a/highlight.c +++ b/highlight.c @@ -533,14 +533,9 @@ void highlight_shell( wchar_t * buff, void *context; wchar_t *cmd=0; - if( !buff || !color ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( buff, ); + CHECK( color, ); - len = wcslen(buff); if( !len ) diff --git a/parse_util.c b/parse_util.c index ce02d8354..2209c3a6f 100644 --- a/parse_util.c +++ b/parse_util.c @@ -52,7 +52,10 @@ int parse_util_lineno( const wchar_t *str, int len ) static wchar_t *prev_str2 = 0; static int i2 = 0; static int res2 = 1; + + CHECK( str, 0 ); + if( str != prev_str || i>len ) { if( prev_str2 == str && i2 <= len ) @@ -100,6 +103,8 @@ int parse_util_locate_cmdsubst( const wchar_t *in, wchar_t *paran_begin=0, *paran_end=0; + CHECK( in, 0 ); + for( pos = (wchar_t *)in; *pos; pos++ ) { if( prev != '\\' ) @@ -176,14 +181,13 @@ void parse_util_cmdsubst_extent( const wchar_t *buff, wchar_t *begin, *end; wchar_t *pos; + CHECK( buff, ); + if( a ) *a=0; if( b ) *b = 0; - if( !buff ) - return; - pos = (wchar_t *)buff; while( 1 ) @@ -243,6 +247,8 @@ static void job_or_process_extent( const wchar_t *buff, tokenizer tok; + CHECK( buff, ); + if( a ) *a=0; if( b ) @@ -342,6 +348,8 @@ void parse_util_token_extent( const wchar_t *buff, wchar_t *a, *b, *pa, *pb; + CHECK( buff, ); + assert( cursor_pos >= 0 ); diff --git a/parser.c b/parser.c index 96a9b472f..d472ffb40 100644 --- a/parser.c +++ b/parser.c @@ -758,12 +758,8 @@ wchar_t *parser_get_filename( void *context, const wchar_t *cmd ) { wchar_t *path; - if( !cmd ) - { - debug( 0, L"%s called with null input", __func__ ); - return 0; - } - + CHECK( cmd, 0 ); + debug( 3, L"parser_get_filename( '%ls' )", cmd ); if(wcschr( cmd, L'/' ) != 0 ) @@ -1022,12 +1018,9 @@ int eval_args( const wchar_t *line, array_list_t *args ) int previous_pos=current_tokenizer_pos; int do_loop=1; - if( !line || !args ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( line, 1 ); + CHECK( args, 1 ); + proc_push_interactive(0); current_tokenizer = &tok; current_tokenizer_pos = 0; @@ -1103,12 +1096,8 @@ void parser_stack_trace( block_t *b, string_buffer_t *buff) /* Validate input */ - if( !buff ) - { - debug( 0, L"%s called with null input", __func__ ); - return; - } - + CHECK( buff, ); + /* Check if we should end the recursion */ @@ -2738,12 +2727,8 @@ static int parser_test_argument( const wchar_t *arg, string_buffer_t *out, const wchar_t *arg_cpy; int do_loop = 1; - if( !arg ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( arg, 1 ); + arg_cpy = wcsdup( arg ); while( do_loop ) @@ -2910,12 +2895,8 @@ int parser_test_args(const wchar_t * buff, int do_loop = 1; int err = 0; - if( !buff ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( buff, 1 ); + current_tokenizer = &tok; for( tok_init( &tok, buff, 0 ); @@ -3008,12 +2989,8 @@ int parser_test( const wchar_t * buff, int arg_count=0; wchar_t *cmd=0; - if( !buff ) - { - debug( 0, L"%s called with null input", __func__ ); - return 1; - } - + CHECK( buff, 1 ); + context = halloc( 0, 0 ); current_tokenizer = &tok; diff --git a/tokenizer.c b/tokenizer.c index 92859d6a9..3b44ae0ab 100644 --- a/tokenizer.c +++ b/tokenizer.c @@ -29,18 +29,16 @@ Error string for unexpected end of string */ #define EOL_ERROR _( L"Unexpected end of token" ) + /** Error string for mismatched parenthesis */ #define PARAN_ERROR _( L"Parenthesis mismatch" ) + /** Error string for invalid redirections */ #define REDIRECT_ERROR _( L"Invalid redirection" ) -/** - Error string for invalid input -*/ -#define INPUT_ERROR _( L"Invalid input" ) /** Error string for when trying to pipe from fd 0 @@ -118,8 +116,9 @@ static void tok_error( tokenizer *tok, const wchar_t *err ) void tok_init( tokenizer *tok, const wchar_t *b, int flags ) { -// fwprintf( stderr, L"CREATE: \'%ls\'\n", b ); + CHECK( tok, ); + CHECK( b, ); memset( tok, 0, sizeof( tokenizer) ); @@ -127,17 +126,6 @@ void tok_init( tokenizer *tok, const wchar_t *b, int flags ) tok->show_comments = flags & TOK_SHOW_COMMENTS; tok->has_next=1; - /* - Before we copy the buffer we need to check that it is not - null. But before that, we need to init the tokenizer far enough - so that errors can be properly flagged - */ - if( !b ) - { - tok_error( tok, INPUT_ERROR ); - return; - } - tok->has_next = (*b != L'\0'); tok->orig_buff = tok->buff = (wchar_t *)(b); @@ -165,6 +153,8 @@ void tok_init( tokenizer *tok, const wchar_t *b, int flags ) void tok_destroy( tokenizer *tok ) { + CHECK( tok, ); + free( tok->last ); if( tok->free_orig ) free( tok->orig_buff ); @@ -172,11 +162,15 @@ void tok_destroy( tokenizer *tok ) int tok_last_type( tokenizer *tok ) { + CHECK( tok, 0 ); + return tok->last_type; } wchar_t *tok_last( tokenizer *tok ) { + CHECK( tok, 0 ); + return tok->last; } @@ -470,6 +464,8 @@ static void read_redirect( tokenizer *tok, int fd ) wchar_t tok_last_quote( tokenizer *tok ) { + CHECK( tok, 0 ); + return tok->last_quote; } @@ -488,15 +484,19 @@ static int my_iswspace( wchar_t c ) const wchar_t *tok_get_desc( int type ) { - + if( type < 0 || type >= sizeof( tok_desc ) ) + { + return _(L"Invalid token type"); + } return _(tok_desc[type]); } void tok_next( tokenizer *tok ) { -// fwprintf( stderr, L"tok_next on %ls (prev=%ls)\n", tok->orig_buff, tok_desc[tok->last_type] ); + CHECK( tok, ); + if( tok_last_type( tok ) == TOK_ERROR ) { tok->has_next=0; @@ -619,6 +619,8 @@ wchar_t *tok_first( const wchar_t *str ) tokenizer t; wchar_t *res=0; + CHECK( str, 0 ); + tok_init( &t, str, 0 ); switch( tok_last_type( &t ) ) @@ -638,12 +640,16 @@ wchar_t *tok_first( const wchar_t *str ) int tok_get_pos( tokenizer *tok ) { + CHECK( tok, 0 ); + return tok->last_pos + (tok->free_orig?1:0); } void tok_set_pos( tokenizer *tok, int pos ) { + CHECK( tok, ); + tok->buff = tok->orig_buff + pos; tok->has_next = 1; tok_next( tok );