vlc security: As i've seen very little improvement on that front, i've decided...

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 <a mrl> :sout=#transcode{vcodec=mp1v,vb=1024,acodec=mpga,ab=192}:standard{mux=ts,dst=vlc-output.ts,access=file}"

Enjoy,
   Damien

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:

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 );
 

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();

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 );

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 );

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 );

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 );

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();

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,

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 );

src/config/chain.c

@@ -30,6 +30,8 @@
 #include <vlc/vlc.h>
 #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:
+                        ;
+                }
+            }
         }
         /* </Check if the option is deprecated> */
 

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 );

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. */

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;
     }