From: Damien Fouilleul Date: Wed, 16 Jan 2008 19:30:12 +0000 (+0000) Subject: vlc security: As i've seen very little improvement on that front, i've decided to... X-Git-Tag: 0.9.0-test0~3423 X-Git-Url: https://git.sesse.net/?a=commitdiff_plain;h=658b4f830b832b19a6de708764f91e1398e501a1;p=vlc vlc security: As i've seen very little improvement on that front, i've decided to check in my take on handling the problem of managing harmful options. I'm pretty sure this is going to be very controversial, but I think my approach is quite simple and yet very effective Anyway, my approach makes the following assumptions: - most vlc options are considered safe, only a handful are particularily unsafe and need be declared as such in their definition (they mostly deal with writing to an output file or URL) - unsafe options are only considered potentially harmful when used as an input option, ie. the ':option' format. Configuration options are always considered safe 'i.e --option' - unsafe options are associated with a global security policy, which dictates how these are handled. At the moment, The policy can be either block, allow or prompt, and is set using the '--security-policy' option (which itself is considered unsafe ;) the policy can be set by the user at the command line or in the preferences, it curently defaults to prompt, which is the desirable state for deskop use. However, it can be overriden depending on context, for example, the activex and mozilla will force the security-policy to block regardless of preference settins. the code is a bit rough at the moment, but i will optimize/clean it up if the dev community this approach is worth keeping. try the following example, and you'll see quickly what i mean: ./vlc -vvv :sout=#transcode{vcodec=mp1v,vb=1024,acodec=mpga,ab=192}:standard{mux=ts,dst=vlc-output.ts,access=file}" Enjoy, Damien --- diff --git a/include/vlc_configuration.h b/include/vlc_configuration.h index 8894b407f2..9432b32016 100644 --- a/include/vlc_configuration.h +++ b/include/vlc_configuration.h @@ -191,7 +191,7 @@ struct module_config_t vlc_bool_t b_autosave; /* Config will be auto-saved at exit time */ vlc_bool_t b_unsaveable; /* Config should be saved */ - vlc_bool_t b_safe; + vlc_bool_t b_unsafe; }; /***************************************************************************** @@ -282,7 +282,7 @@ enum vlc_config_properties VLC_CONFIG_OLDNAME, /* former option name (args=const char *) */ - VLC_CONFIG_SAFE, + VLC_CONFIG_UNSAFE, /* tag as modifiable by untrusted input item "sources" (args=none) */ }; @@ -472,8 +472,8 @@ VLC_EXPORT( int, vlc_config_set, (module_config_t *, int, ...) ); #define change_unsaveable() \ vlc_config_set (p_config, VLC_CONFIG_VOLATILE) -#define change_safe() \ - vlc_config_set (p_config, VLC_CONFIG_SAFE) +#define change_unsafe() \ + vlc_config_set (p_config, VLC_CONFIG_UNSAFE) /**************************************************************************** * config_chain_t: diff --git a/modules/access_filter/record.c b/modules/access_filter/record.c index 6ae910a881..b9585a185d 100644 --- a/modules/access_filter/record.c +++ b/modules/access_filter/record.c @@ -57,6 +57,7 @@ vlc_module_begin(); add_directory( "record-path", NULL, NULL, RECORD_PATH_TXT, RECORD_PATH_LONGTXT, VLC_TRUE ); + change_unsafe(); set_callbacks( Open, Close ); diff --git a/modules/access_filter/timeshift.c b/modules/access_filter/timeshift.c index fd141c8168..788ca3d700 100644 --- a/modules/access_filter/timeshift.c +++ b/modules/access_filter/timeshift.c @@ -65,6 +65,7 @@ vlc_module_begin(); add_integer( "timeshift-granularity", 50, NULL, GRANULARITY_TEXT, GRANULARITY_LONGTEXT, VLC_TRUE ); add_directory( "timeshift-dir", 0, 0, DIR_TEXT, DIR_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_bool( "timeshift-force", VLC_FALSE, NULL, FORCE_TEXT, FORCE_LONGTEXT, VLC_FALSE ); vlc_module_end(); diff --git a/modules/audio_output/file.c b/modules/audio_output/file.c index 5ac85e353c..788671ff4a 100644 --- a/modules/audio_output/file.c +++ b/modules/audio_output/file.c @@ -116,6 +116,7 @@ vlc_module_begin(); CHANNELS_TEXT, CHANNELS_LONGTEXT, VLC_TRUE ); add_file( "audiofile-file", "audiofile.wav", NULL, FILE_TEXT, FILE_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_bool( "audiofile-wav", 1, NULL, WAV_TEXT, WAV_LONGTEXT, VLC_TRUE ); set_capability( "audio output", 0 ); diff --git a/modules/demux/demuxdump.c b/modules/demux/demuxdump.c index 7023ffbd9a..d1a3fc552c 100644 --- a/modules/demux/demuxdump.c +++ b/modules/demux/demuxdump.c @@ -51,6 +51,7 @@ vlc_module_begin(); set_capability( "demux2", 0 ); add_file( "demuxdump-file", "stream-demux.dump", NULL, FILE_TEXT, FILE_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_bool( "demuxdump-append", 0, NULL, APPEND_TEXT, APPEND_LONGTEXT, VLC_FALSE ); set_callbacks( Open, Close ); diff --git a/modules/demux/ts.c b/modules/demux/ts.c index 7c9d52d30d..817a69fd81 100644 --- a/modules/demux/ts.c +++ b/modules/demux/ts.c @@ -148,6 +148,7 @@ vlc_module_begin(); add_bool( "ts-silent", 0, NULL, SILENT_TEXT, SILENT_LONGTEXT, VLC_TRUE ); add_file( "ts-dump-file", NULL, NULL, TSDUMP_TEXT, TSDUMP_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_bool( "ts-dump-append", 0, NULL, APPEND_TEXT, APPEND_LONGTEXT, VLC_FALSE ); add_integer( "ts-dump-size", 16384, NULL, DUMPSIZE_TEXT, DUMPSIZE_LONGTEXT, VLC_TRUE ); diff --git a/modules/misc/logger.c b/modules/misc/logger.c index 4f50d34398..61cb45a54a 100644 --- a/modules/misc/logger.c +++ b/modules/misc/logger.c @@ -135,6 +135,7 @@ vlc_module_begin(); add_file( "logfile", NULL, NULL, N_("Log filename"), N_("Specify the log filename."), VLC_FALSE ); + change_unsafe(); add_string( "logmode", "text", NULL, LOGMODE_TEXT, LOGMODE_LONGTEXT, VLC_FALSE ); change_string_list( mode_list, mode_list_text, 0 ); diff --git a/modules/stream_out/es.c b/modules/stream_out/es.c index 33defdadd8..99879473c9 100644 --- a/modules/stream_out/es.c +++ b/modules/stream_out/es.c @@ -93,10 +93,13 @@ vlc_module_begin(); add_string( SOUT_CFG_PREFIX "dst", "", NULL, DEST_TEXT, DEST_LONGTEXT, VLC_TRUE ); + change_unsafe(); add_string( SOUT_CFG_PREFIX "dst-audio", "", NULL, DESTA_TEXT, DESTA_LONGTEXT, VLC_TRUE ); + change_unsafe(); add_string( SOUT_CFG_PREFIX "dst-video", "", NULL, DESTV_TEXT, DESTV_LONGTEXT, VLC_TRUE ); + change_unsafe(); set_callbacks( Open, Close ); vlc_module_end(); diff --git a/modules/stream_out/rtp.c b/modules/stream_out/rtp.c index 7a2f28e9ec..c4228971e2 100644 --- a/modules/stream_out/rtp.c +++ b/modules/stream_out/rtp.c @@ -147,6 +147,7 @@ vlc_module_begin(); add_string( SOUT_CFG_PREFIX "dst", "", NULL, DST_TEXT, DST_LONGTEXT, VLC_TRUE ); + change_unsafe(); add_string( SOUT_CFG_PREFIX "sdp", "", NULL, SDP_TEXT, SDP_LONGTEXT, VLC_TRUE ); add_string( SOUT_CFG_PREFIX "mux", "", NULL, MUX_TEXT, diff --git a/modules/stream_out/standard.c b/modules/stream_out/standard.c index 2f4f5d7bc3..3f50459e81 100644 --- a/modules/stream_out/standard.c +++ b/modules/stream_out/standard.c @@ -95,6 +95,7 @@ vlc_module_begin(); MUX_LONGTEXT, VLC_FALSE ); add_string( SOUT_CFG_PREFIX "dst", "", NULL, DST_TEXT, DST_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_bool( SOUT_CFG_PREFIX "sap", VLC_FALSE, NULL, SAP_TEXT, SAP_LONGTEXT, VLC_TRUE ); diff --git a/src/config/chain.c b/src/config/chain.c index 4d71bd5c6e..53cd9e3511 100644 --- a/src/config/chain.c +++ b/src/config/chain.c @@ -30,6 +30,8 @@ #include #include "libvlc.h" +#include "vlc_interface.h" + /***************************************************************************** * Local prototypes *****************************************************************************/ @@ -314,6 +316,30 @@ void __config_ChainParse( vlc_object_t *p_this, const char *psz_prefix, msg_Warn( p_this, "Option %s is obsolete. Use %s instead.", name, psz_name ); } + if( p_conf->b_unsafe ) + { + int policy = config_GetInt( p_this, "security-policy" ); + switch( policy ) + { + case 0: /* block */ + msg_Err( p_this, "option %s is unsafe and is blocked by security policy", psz_name ); + return; + case 1: /* allow */ + break; + case 2: /* prompt */ + { + char description[256]; + snprintf(description, sizeof(description), _("playlist item is making use of the following unsafe option '%s', which may be harmful if used in a malicious way, authorize it ?"), psz_name); + if( DIALOG_OK_YES != intf_UserYesNo( p_this, _("WARNING: Unsafe Playlist"), description, _("Yes"), _("No"), NULL) ) + { + msg_Err( p_this, "option %s is unsafe and is blocked by security policy", psz_name ); + return; + } + } + default: + ; + } + } } /* */ diff --git a/src/libvlc-module.c b/src/libvlc-module.c index 0d655a6da7..156de1df17 100644 --- a/src/libvlc-module.c +++ b/src/libvlc-module.c @@ -977,6 +977,14 @@ static const char *ppsz_clock_descriptions[] = #define MINIMIZE_THREADS_LONGTEXT N_( \ "This option minimizes the number of threads needed to run VLC.") +#define SECURITY_POLICY_TEXT N_("Policy for handling unsafe options.") +#define SECURITY_POLICY_LONGTEXT N_( \ + "This option dictates the default policy when processing options " \ + "which may be harmful when used in a malicious way.") + +static int pi_secpolicy_values[] = { 0, 1, 2 }; +static const char *ppsz_secpolicy_descriptions[] = { N_("Block"), N_("Allow"), N_("Prompt") }; + #define PLUGIN_PATH_TEXT N_("Modules search path") #define PLUGIN_PATH_LONGTEXT N_( \ "Additional path for VLC to look for its modules.") @@ -1481,6 +1489,7 @@ vlc_module_begin(); set_section( N_("Snapshot") , NULL ); add_directory( "snapshot-path", NULL, NULL, SNAP_PATH_TEXT, SNAP_PATH_LONGTEXT, VLC_FALSE ); + change_unsafe(); add_string( "snapshot-prefix", "vlcsnap-", NULL, SNAP_PREFIX_TEXT, SNAP_PREFIX_LONGTEXT, VLC_FALSE ); add_string( "snapshot-format", "png", NULL, SNAP_FORMAT_TEXT, @@ -1786,12 +1795,20 @@ vlc_module_begin(); add_directory( "plugin-path", NULL, NULL, PLUGIN_PATH_TEXT, PLUGIN_PATH_LONGTEXT, VLC_TRUE ); change_need_restart(); + change_unsafe(); set_section( N_("Performance options"), NULL ); add_bool( "minimize-threads", 0, NULL, MINIMIZE_THREADS_TEXT, MINIMIZE_THREADS_LONGTEXT, VLC_TRUE ); change_need_restart(); + set_section( N_("Security options"), NULL ); + add_integer( "security-policy", 2, NULL, SECURITY_POLICY_TEXT, + SECURITY_POLICY_LONGTEXT, VLC_TRUE ); + change_integer_list( pi_secpolicy_values, ppsz_secpolicy_descriptions, 0 ); + change_unsafe(); + change_need_restart(); + #if !defined(__APPLE__) && !defined(SYS_BEOS) && defined(PTHREAD_COND_T_IN_PTHREAD_H) add_bool( "rt-priority", VLC_FALSE, NULL, RT_PRIORITY_TEXT, RT_PRIORITY_LONGTEXT, VLC_TRUE ); diff --git a/src/misc/variables.c b/src/misc/variables.c index 2719da61d1..0a53ab85c0 100644 --- a/src/misc/variables.c +++ b/src/misc/variables.c @@ -29,6 +29,8 @@ #include "libvlc.h" +#include "vlc_interface.h" + /***************************************************************************** * Private types *****************************************************************************/ @@ -1090,6 +1092,35 @@ void __var_OptionParse( vlc_object_t *p_obj, const char *psz_option ) if( ( i_type != VLC_VAR_BOOL ) && ( !psz_value || !*psz_value ) ) goto cleanup; /* Invalid value */ + /* check if option is unsafe */ + { + module_config_t *p_config = config_FindConfig( p_obj, psz_name ); + if( p_config->b_unsafe ) + { + int policy = config_GetInt( p_obj, "security-policy" ); + switch( policy ) + { + case 0: /* block */ + msg_Err( p_obj, "option %s is unsafe and is blocked by security policy", psz_name ); + return; + case 1: /* allow */ + break; + case 2: /* prompt */ + { + char description[256]; + snprintf(description, sizeof(description), _("playlist item is making use of the following unsafe option '%s', which may be harmful if used in a malicious way, authorize it ?"), psz_name); + if( DIALOG_OK_YES != intf_UserYesNo( p_obj, _("WARNING: Unsafe Playlist"), description, _("Yes"), _("No"), NULL) ) + { + msg_Err( p_obj, "option %s is unsafe and is blocked by security policy", psz_name ); + return; + } + } + default: + ; + } + } + } + /* Create the variable in the input object. * Children of the input object will be able to retreive this value * thanks to the inheritance property of the object variables. */ diff --git a/src/modules/entry.c b/src/modules/entry.c index ded237248d..ac006c9b23 100644 --- a/src/modules/entry.c +++ b/src/modules/entry.c @@ -407,8 +407,8 @@ int vlc_config_set (module_config_t *restrict item, int id, ...) break; } - case VLC_CONFIG_SAFE: - item->b_safe = VLC_TRUE; + case VLC_CONFIG_UNSAFE: + item->b_unsafe = VLC_TRUE; ret = 0; break; }