Message ID | 20170425234752.67011-1-lq@chinaffmpeg.org |
---|---|
State | Superseded |
Headers | show |
> On Apr 25, 2017, at 18:47, Steven Liu <lq@chinaffmpeg.org> wrote: > > CID: 1405135 I have no idea what this message is supposed to mean. > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 3ec0f330fb..b7aafb73da 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -394,11 +394,11 @@ static int do_encrypt(AVFormatContext *s) > av_strlcat(hls->key_basename, ".key", len); > > if (hls->key_url) { > - strncpy(hls->key_file, hls->key_url, sizeof(hls->key_file)); > - strncpy(hls->key_uri, hls->key_url, sizeof(hls->key_uri)); > + av_strlcpy(hls->key_file, hls->key_url, strlen(hls->key_url)); > + av_strlcpy(hls->key_uri, hls->key_url, strlen(hls->key_url)); Changing this to av_strlcpy makes sense, but using strlen() here replaces a DoS vulnerability with a memory-corruption one. Use sizeof() like the original code in all 4 cases. > } else { > - strncpy(hls->key_file, hls->key_basename, sizeof(hls->key_file)); > - strncpy(hls->key_uri, hls->key_basename, sizeof(hls->key_uri)); > + av_strlcpy(hls->key_file, hls->key_basename, strlen(hls->key_basename)); > + av_strlcpy(hls->key_uri, hls->key_basename, strlen(hls->key_basename)); > } > > if (!*hls->iv_string) { > -- > 2.11.0 (Apple Git-81) > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On 4/25/2017 4:47 PM, Steven Liu wrote: > CID: 1405135 > Signed-off-by: Steven Liu <lq@chinaffmpeg.org> > --- > libavformat/hlsenc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c > index 3ec0f330fb..b7aafb73da 100644 > --- a/libavformat/hlsenc.c > +++ b/libavformat/hlsenc.c > @@ -394,11 +394,11 @@ static int do_encrypt(AVFormatContext *s) > av_strlcat(hls->key_basename, ".key", len); > > if (hls->key_url) { > - strncpy(hls->key_file, hls->key_url, sizeof(hls->key_file)); > - strncpy(hls->key_uri, hls->key_url, sizeof(hls->key_uri)); > + av_strlcpy(hls->key_file, hls->key_url, strlen(hls->key_url)); > + av_strlcpy(hls->key_uri, hls->key_url, strlen(hls->key_url)); > } else { > - strncpy(hls->key_file, hls->key_basename, sizeof(hls->key_file)); > - strncpy(hls->key_uri, hls->key_basename, sizeof(hls->key_uri)); > + av_strlcpy(hls->key_file, hls->key_basename, strlen(hls->key_basename)); > + av_strlcpy(hls->key_uri, hls->key_basename, strlen(hls->key_basename)); If either hls->key_url or hls->key_basename, depending on which path the code takes, is longer in length than key_file or key_uri (both of size LINE_BUFFER_SIZE + 1), then the calls to av_strlcpy() will write past the end of the buffers. As key_url corresponds to a user-passed option, and I think the same may be true for key_basename based on the way it is formed, this in theory could be a problem, although it is unlikely in practice since LINE_BUFFER_SIZE is 1024. While ideally there would be a sanity check somewhere in the code to make sure those URLs aren't too long, an alternative is to copy up to max LINE_BUFFER_SIZE characters and make sure the string is null-terminated in that case. Aaron Levinson
diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c index 3ec0f330fb..b7aafb73da 100644 --- a/libavformat/hlsenc.c +++ b/libavformat/hlsenc.c @@ -394,11 +394,11 @@ static int do_encrypt(AVFormatContext *s) av_strlcat(hls->key_basename, ".key", len); if (hls->key_url) { - strncpy(hls->key_file, hls->key_url, sizeof(hls->key_file)); - strncpy(hls->key_uri, hls->key_url, sizeof(hls->key_uri)); + av_strlcpy(hls->key_file, hls->key_url, strlen(hls->key_url)); + av_strlcpy(hls->key_uri, hls->key_url, strlen(hls->key_url)); } else { - strncpy(hls->key_file, hls->key_basename, sizeof(hls->key_file)); - strncpy(hls->key_uri, hls->key_basename, sizeof(hls->key_uri)); + av_strlcpy(hls->key_file, hls->key_basename, strlen(hls->key_basename)); + av_strlcpy(hls->key_uri, hls->key_basename, strlen(hls->key_basename)); } if (!*hls->iv_string) {
CID: 1405135 Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hlsenc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)