[Spice-devel] [vdagent-win PATCH v3 04/10] Fix some minor buffer overflows reading registry informations

[Jonathon Jongsma]

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Fri, 2018-06-29 at 08:11 +0100, Frediano Ziglio wrote:
> Strings in the registry can be not NUL-terminated.
> Current code to make sure they are NUL-terminated can add an extra
> NUL character at the end of the buffer.
> Also RegQueryValueEx returns the number of bytes read, not the number
> of characters so the value must be fixed to avoid overflows.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  vdagent/display_setting.cpp | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/vdagent/display_setting.cpp
> b/vdagent/display_setting.cpp
> index 25a248e..2b22144 100644
> --- a/vdagent/display_setting.cpp
> +++ b/vdagent/display_setting.cpp
> @@ -285,7 +285,7 @@ bool DisplaySetting::disable_wallpaper()
>  bool DisplaySetting::reload_wallpaper(HKEY desktop_reg_key)
>  {
>      TCHAR wallpaper_path[MAX_PATH + 1];
> -    DWORD value_size = sizeof(wallpaper_path);
> +    DWORD value_size = sizeof(wallpaper_path) -
> sizeof(wallpaper_path[0]);
>      DWORD value_type;
>      LONG status;
>      TCHAR cur_wallpaper[MAX_PATH + 1];
> @@ -303,7 +303,8 @@ bool DisplaySetting::reload_wallpaper(HKEY
> desktop_reg_key)
>          return false;
>      }
>  
> -    if (wallpaper_path[value_size - 1] != '\0') {
> +    value_size /= sizeof(wallpaper_path[0]);
> +    if (!value_size || wallpaper_path[value_size - 1] != '\0') {
>          wallpaper_path[value_size] = '\0';
>      }
>  
> @@ -339,7 +340,7 @@ bool DisplaySetting::disable_font_smoothing()
>  bool DisplaySetting::reload_font_smoothing(HKEY desktop_reg_key)
>  {
>      CHAR smooth_value[4];
> -    DWORD value_size = sizeof(smooth_value);
> +    DWORD value_size = sizeof(smooth_value)-1;
>      DWORD value_type;
>      LONG status;
>      BOOL cur_font_smooth;
> @@ -357,7 +358,7 @@ bool DisplaySetting::reload_font_smoothing(HKEY
> desktop_reg_key)
>          return false;
>      }
>  
> -    if (smooth_value[value_size - 1] != '\0') {
> +    if (!value_size || smooth_value[value_size - 1] != '\0') {
>          smooth_value[value_size] = '\0';
>      }
>  
> @@ -412,7 +413,7 @@ bool DisplaySetting::reload_win_animation(HKEY
> desktop_reg_key)
>  {
>      HKEY win_metrics_hkey;
>      CHAR win_anim_value[4];
> -    DWORD value_size = sizeof(win_anim_value);
> +    DWORD value_size = sizeof(win_anim_value)-1;
>      DWORD value_type;
>      LONG status;
>      ANIMATIONINFO active_win_animation;
> @@ -441,7 +442,7 @@ bool DisplaySetting::reload_win_animation(HKEY
> desktop_reg_key)
>          return false;
>      }
>  
> -    if (win_anim_value[value_size - 1] != '\0') {
> +    if (!value_size || win_anim_value[value_size - 1] != '\0') {
>          win_anim_value[value_size] = '\0';
>      }
>