[vlc-devel] [vlc-commits] service_discovery: sap: fix potential invalid write.

[Rémi Denis-Courmont]

Le vendredi 05 décembre 2014, 23:23:04 Fabian Yamaguchi a écrit :
> vlc | branch: master | Fabian Yamaguchi <fyamagu at gwdg.de> | Fri Dec  5
> 14:24:04 2014 +0100| [85b463d526df220ffb347ae7cf3475b86392195b] |
> committer: Jean-Baptiste Kempf
> 
> service_discovery: sap: fix potential invalid write.
> 
> A buffer depending on the length of psz_sdp was allocated on the
> stack, and therefore, allocation failure would not be
> detected. Allocate the buffer on the heap instead and check for
> allocation failure to avoid a potential invalid memory access in the
> subsequent memcpy operation.
> 
> Signed-off-by: Jean-Baptiste Kempf <jb at videolan.org>
> 
> > http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=85b463d526df220ffb3
> > 47ae7cf3475b86392195b
> ---
> 
>  modules/services_discovery/sap.c |   19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/modules/services_discovery/sap.c
> b/modules/services_discovery/sap.c index 8888215..ce6e9fa 100644
> --- a/modules/services_discovery/sap.c
> +++ b/modules/services_discovery/sap.c
> @@ -1184,6 +1184,7 @@ static sdp_t *ParseSDP (vlc_object_t *p_obj, const
> char *psz_sdp) socklen_t glob_len = 0;
>      unsigned glob_count = 1;
>      int port = 0;
> +    char *line = NULL;
> 
>      /* TODO: use iconv and charset attribute instead of EnsureUTF8 */
>      while (*psz_sdp)
> @@ -1191,7 +1192,19 @@ static sdp_t *ParseSDP (vlc_object_t *p_obj, const
> char *psz_sdp) /* Extract one line */
>          char *eol = strchr (psz_sdp, '\n');
>          size_t linelen = eol ? (size_t)(eol - psz_sdp) : strlen (psz_sdp);
> -        char line[linelen + 1];
> +
> +        if (linelen == SIZE_MAX)

The code does not allow SDP larger than 64 KiB or thereabout. You would have 
found that out if you had actually read the code.

But I digress. How any string *ever* be SIZE_MAX-bytes long?

This commit is _obviously_ non-sense and I don´t understand how it even got 
pushed.

-- 
Rémi Denis-Courmont
http://www.remlab.net/