[vlc-commits] [Git][videolan/vlc][master] 8 commits: cli: fix possible uninitialized read

Steve Lhomme (@robUx4) gitlab at videolan.org
Fri Jan 26 12:15:24 UTC 2024



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
b7eeec2c by Romain Vimont at 2024-01-26T11:43:07+00:00
cli: fix possible uninitialized read

On Android:

    ../../modules/control/cli/cli.c:335:9: warning: variable 'ret' is
            used uninitialized whenever 'if' condition is false
            [-Wsometimes-uninitialized]
        if (count > 0)
            ^~~~~~~~~
    ../../modules/control/cli/cli.c:359:12: note: uninitialized uses
            occurs here
        return ret;
               ^~~

Return an error if count == 0 in the `#ifdef HAVE_WORDEXP` branch, and
replace the if-condition by an assertion.

- - - - -
f4ce1157 by Steve Lhomme at 2024-01-26T11:43:07+00:00
cli: fix unused warnings

- - - - -
ce70f053 by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: switch to signed iov len for `cli_writev`

Both `vlc_writev` and `writev` take signed parameters to describe the
count of buffers to write.

- - - - -
933ce085 by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: replace `atof` with `strtof`

`strtof` directly parses into a float (`atof` actually parses into
doubles) and allows us to handle overflows as ranges error.

- - - - -
32fb4902 by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: refuse wrong volume values

Disallow values that aren't between 0. and 2. as the audio output
documentation requires it.

- - - - -
fd72da1b by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: fix various unnecessary narrowing conversions

- - - - -
0abc313d by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: remove incorrect unused mention

The variable is actually used in the function body.

- - - - -
fff2ccbe by Alaric Senat at 2024-01-26T11:43:07+00:00
cli: remove unused variable

The volume value isn't used anywhere and specifying NULL is allowed by
`vlc_player_aout_IncrementVolume`.

- - - - -


2 changed files:

- modules/control/cli/cli.c
- modules/control/cli/player.c


Changes:

=====================================
modules/control/cli/cli.c
=====================================
@@ -308,6 +308,11 @@ error:      wordfree(&we);
     }
 
     size_t count = we.we_wordc;
+    if (count == 0) {
+        ret = VLC_EGENERIC;
+        goto error;
+    }
+
     const char **args = vlc_alloc(count, sizeof (*args));
     if (unlikely(args == NULL))
     {
@@ -337,22 +342,19 @@ error:      wordfree(&we);
     }
 #endif
 
-    if (count > 0)
-    {
-        const struct command **pp = tfind(&args[0], &sys->commands, cmdcmp);
+    assert(count > 0);
+    const struct command **pp = tfind(&args[0], &sys->commands, cmdcmp);
 
-        if (pp != NULL)
-        {
-            const struct command *c = *pp;
-
-            ret = c->handler.callback(cl, args, count, c->data);
-        }
-        else
-        {
-            cli_printf(cl, _("Unknown command `%s'. Type `help' for help."),
-                      args[0]);
-            ret = VLC_EGENERIC;
-        }
+    if (pp != NULL)
+    {
+        const struct command *c = *pp;;
+        ret = c->handler.callback(cl, args, count, c->data);
+    }
+    else
+    {
+        cli_printf(cl, _("Unknown command `%s'. Type `help' for help."),
+                  args[0]);
+        ret = VLC_EGENERIC;
     }
 
 #ifdef HAVE_WORDEXP
@@ -366,7 +368,7 @@ error:      wordfree(&we);
 
 #ifndef _WIN32
 static ssize_t cli_writev(struct cli_client *cl,
-                          const struct iovec *iov, unsigned iovlen)
+                          const struct iovec *iov, int iovlen)
 {
     ssize_t val;
 
@@ -827,7 +829,9 @@ static void *Run( void *data )
 static int Activate( vlc_object_t *p_this )
 {
     intf_thread_t *p_intf = (intf_thread_t*)p_this;
+#ifndef _WIN32
     struct cli_client *cl;
+#endif
     char *psz_host;
     int  *pi_socket = NULL;
 


=====================================
modules/control/cli/player.c
=====================================
@@ -113,7 +113,7 @@ player_on_rate_changed(vlc_player_t *player, float new_rate, void *data)
 static void
 player_on_position_changed(vlc_player_t *player,
                            vlc_tick_t new_time, double new_pos, void *data)
-{ VLC_UNUSED(player); VLC_UNUSED(new_pos);
+{ VLC_UNUSED(player);
     struct player_cli *pc = data;
 
     if (pc->input_buffering)
@@ -123,7 +123,7 @@ player_on_position_changed(vlc_player_t *player,
         pc->input_buffering = false;
     }
 
-    long position = lroundf(new_pos * 100.f);
+    long position = lround(new_pos * 100.);
 
     if (pc->show_position && position != pc->position)
     {
@@ -154,6 +154,18 @@ static const struct vlc_player_aout_cbs player_aout_cbs =
     .on_volume_changed = player_aout_on_volume_changed,
 };
 
+static int ParseFloat(const char *str, float *out)
+{
+    char *end;
+    *out = strtof(str, &end);
+    if (*end != '\0' && *out == 0.f)
+        return -EINVAL; /* No valid data parsed. */
+    if (unlikely(!isfinite(*out)))
+        return -EINVAL; /* Invalid values in this context. */
+    return VLC_SUCCESS;
+}
+
+
 static int PlayerDoVoid(struct cli_client *cl, void *data,
                         void (*cb)(vlc_player_t *))
 {
@@ -181,8 +193,13 @@ static int PlayerDoFloat(struct cli_client *cl, const char *const *args,
             cli_printf(cl, "%f", getter(player));
             break;
         case 2:
-            setter(player, atof(args[1]));
+        {
+            float v;
+            ret = ParseFloat(args[1], &v);
+            if (ret == VLC_SUCCESS)
+                setter(player, v);
             break;
+        }
         default:
             ret = VLC_EGENERIC; /* EINVAL */
     }
@@ -584,7 +601,6 @@ static int Volume(struct cli_client *cl, const char *const *args, size_t count,
 {
     vlc_player_t *player = data;
 
-    vlc_player_Lock(player);
     if (count == 2)
     {
         /* NOTE: For unfortunate hysterical raisins, integer value above 1 are
@@ -602,13 +618,23 @@ static int Volume(struct cli_client *cl, const char *const *args, size_t count,
         if (*end == '\0' && ul > 1 && ul <= AOUT_VOLUME_MAX)
             volume = ldexpf(ul, -stdc_trailing_zeros((unsigned)AOUT_VOLUME_DEFAULT));
         else
-            volume = atof(args[1]);
+        {
+            const int result = ParseFloat(args[1], &volume);
+            if (result != VLC_SUCCESS)
+                return result;
+            if (volume < 0.f || volume > 2.f )
+                return -EINVAL;
+        }
 
+        vlc_player_Lock(player);
         vlc_player_aout_SetVolume(player, volume);
     }
     else
+    {
+        vlc_player_Lock(player);
         cli_printf(cl, STATUS_CHANGE "( audio volume: %f )",
                    vlc_player_aout_GetVolume(player));
+    }
     vlc_player_Unlock(player);
     return 0;
 }
@@ -620,14 +646,13 @@ static int VolumeMove(struct cli_client *cl, const char *const *args,
     const char *psz_cmd = args[0];
     const char *arg = count > 1 ? args[1] : "";
 
-    float volume;
     int i_nb_steps = atoi(arg);
 
     if( !strcmp(psz_cmd, "voldown") )
         i_nb_steps *= -1;
 
     vlc_player_Lock(player);
-    vlc_player_aout_IncrementVolume(player, i_nb_steps, &volume);
+    vlc_player_aout_IncrementVolume(player, i_nb_steps, NULL);
     vlc_player_Unlock(player);
     (void) cl;
     return 0;
@@ -657,7 +682,14 @@ static int VideoConfig(struct cli_client *cl, const char *const *args,
         /* set */
         if( !strcmp( psz_variable, "zoom" ) )
         {
-            float f_float = atof( arg );
+            float f_float;
+            const int r = ParseFloat( arg, &f_float );
+            if( r != VLC_SUCCESS )
+            {
+                vout_Release( p_vout );
+                return r;
+            }
+
             var_SetFloat( p_vout, psz_variable, f_float );
         }
         else
@@ -669,7 +701,7 @@ static int VideoConfig(struct cli_client *cl, const char *const *args,
         char *name;
         vlc_value_t *val;
         char **text;
-        float f_value = 0.;
+        float f_value = 0.f;
         char *psz_value = NULL;
         size_t count;
 
@@ -1038,7 +1070,7 @@ void *RegisterPlayer(intf_thread_t *intf)
         return NULL;
 
     pc->intf = intf;
-    pc->position = -1.;
+    pc->position = -1;
     pc->input_buffering = false;
     pc->show_position = var_InheritBool(intf, "rc-show-pos");
 



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c2ee077679f3a03d2fc7f90e9bcd24f26f4aafbd...fff2ccbe181de563436055606a0aea931775567e

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/c2ee077679f3a03d2fc7f90e9bcd24f26f4aafbd...fff2ccbe181de563436055606a0aea931775567e
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list