Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[Rodger Combs]

I'm not confident that we'll be able to verify the correctness of any 
validation code for a printf format string sufficiently. Just parse out the 
padding value and use "%0*"PRId64. Do not pass anything other than a string 
literal to printf-family functions in future versions of this patch.

> On Aug 30, 2017, at 14:17, samsamsam  wrote:
> 
> No, you have not get the point.
> The hackers won’t refer to specification and just broke the rule to get root 
> shell.
> 
> I got a point. This is why I proposed the validation if validation faile then 
> return error and stop processing. 
> But, I was not checked proposed validation it was only concept. I will write 
> correct validation when I will have access to my PC in the next week.
> 
> 
> Dnia 30 sierpnia 2017 09:59 刘歧  napisał(a):
> 
> 在 2017年8月30日，15:52，samsamsam  写道：
> 
> What will happen when $Number%02s%08d$
> 
> Exacly. Such format is not allowed by dash specification. So, such validation 
> is sufficient.
> 
> Please check DASH specification format like that $Number%02s%08d$ is not 
> allowed.
> So what for you want to allow it?
> No, you have not get the point.
> The hackers won’t refer to specification and just broke the rule to get root 
> shell.
> 
> Anyway, merge your code and will update a new version patch later.
> 
> 
> Dnia 30 sierpnia 2017 09:32 刘歧  napisał(a):
> 
> 在 2017年8月30日，14:38，samsamsam  写道：
> 
> But you do not added this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> What will happen when $Number%02s%08d$
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
> format_len = end - start - marker_len - 1 + strlen(PRId64);
> *o_format = av_mallocz(format_len+1);
> strncpy(*o_format, start + marker_len, end - start - marker_len -1);
> strcat(*o_format, PRId64);
> …
> }
> 
> modification. Right?
> 
> Dnia 30 sierpnia 2017 00:04 Liu Steven  napisał(a):
> 
> 在 2017年8月30日，上午3:30，samsamsam  写道：
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> What? With my solution this is not problem.
> Why you think %s%d%d%d%d or %02c%lld give any problem?
> 
> localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
> ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg 
> developers
> built with Apple LLVM version 8.1.0 (clang-802.0.42)
> configuration: --enable-libass --enable-opengl --enable-libx264 
> --enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
> --enable-libopencv --enable-libtesseract --enable-libspeex 
> --enable-libfreetype --enable-libfontconfig --enable-libfdk-aac 
> --enable-videotoolbox --enable-libxml2
> libavutil  55. 74.100 / 55. 74.100
> libavcodec 57.103.101 / 57.103.101
> libavformat57. 78.100 / 57. 78.100
> libavdevice57.  7.101 / 57.  7.101
> libavfilter 6.100.100 /  6.100.100
> libswscale  4.  7.103 /  4.  7.103
> libswresample   2.  8.100 /  2.  8.100
> libpostproc54.  6.100 / 54.  6.100
> [dash @ 0x7ffb76802a00] 11
> Segmentation fault: 11ted 1 times
> localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
> /tmp/dash.mpd: initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1">
> localhost:dash StevenLiu$
> 
> reproduce:
> ffmpeg -i input -c copy -f dash /tmp/dash.mpd
> vim /tmp/dash.mpd
> seek to `media="chunk-stream$RepresentationID`
> the context is `  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="1”>`
> modify to:
>  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1”>
> 
> use your solution:
> ./ffmpeg -i /tmp/dash.mpd
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 12:27 刘歧  napisał(a):
> 
> 在 2017年8月28日，18:12，samsamsam  写道：
> 
> Validation will be very simple. I am talking about something like this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
> format_len = end - start - marker_len - 1 + strlen(PRId64);
> *o_format = av_mallocz(format_len+1);
> strncpy(*o_format, start + marker_len, end - start - marker_len -1);
> strcat(*o_format, PRId64);
> …
> }
> maybe more complex than this 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[刘歧]

> 在 2017年8月30日，16:15，samsamsam  写道：
> 
> Sorry, my fault. Such simple check proposed by me is incorrect.
> It not handle basic "%05d" format.
That’s no matter samsamsam, if the string is not standard as specification 
describe, the result maybe need give an error.
So that is not a generic template, so fix it later :P
> 
> Dnia 30 sierpnia 2017 09:56 samsamsam  napisał(a):
> 
> What will happen when $Number%02s%08d$
> 
> Error will be returned because this format is not valid in the sense of dash 
> spec.
> 
> Dnia 30 sierpnia 2017 09:39 刘歧  napisał(a):
> 
> 在 2017年8月30日，15:32，刘歧  写道：
> 
> 
> 在 2017年8月30日，14:38，samsamsam  写道：
> 
> But you do not added this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> What will happen when $Number%02s%08d$
> 
> Test passed, now i have use a new function named get_segment_filename and the 
> get_current_fragment,
> that would check the file name ok or not.
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
> format_len = end - start - marker_len - 1 + strlen(PRId64);
> *o_format = av_mallocz(format_len+1);
> strncpy(*o_format, start + marker_len, end - start - marker_len -1);
> strcat(*o_format, PRId64);
> …
> }
> 
> modification. Right?
> 
> Dnia 30 sierpnia 2017 00:04 Liu Steven  napisał(a):
> 
> 在 2017年8月30日，上午3:30，samsamsam  写道：
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> What? With my solution this is not problem.
> Why you think %s%d%d%d%d or %02c%lld give any problem?
> 
> localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
> ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg 
> developers
> built with Apple LLVM version 8.1.0 (clang-802.0.42)
> configuration: --enable-libass --enable-opengl --enable-libx264 
> --enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
> --enable-libopencv --enable-libtesseract --enable-libspeex 
> --enable-libfreetype --enable-libfontconfig --enable-libfdk-aac 
> --enable-videotoolbox --enable-libxml2
> libavutil  55. 74.100 / 55. 74.100
> libavcodec 57.103.101 / 57.103.101
> libavformat57. 78.100 / 57. 78.100
> libavdevice57.  7.101 / 57.  7.101
> libavfilter 6.100.100 /  6.100.100
> libswscale  4.  7.103 /  4.  7.103
> libswresample   2.  8.100 /  2.  8.100
> libpostproc54.  6.100 / 54.  6.100
> [dash @ 0x7ffb76802a00] 11
> Segmentation fault: 11ted 1 times
> localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
> /tmp/dash.mpd: initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1">
> localhost:dash StevenLiu$
> 
> reproduce:
> ffmpeg -i input -c copy -f dash /tmp/dash.mpd
> vim /tmp/dash.mpd
> seek to `media="chunk-stream$RepresentationID`
> the context is `  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="1”>`
> modify to:
>  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1”>
> 
> use your solution:
> ./ffmpeg -i /tmp/dash.mpd
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 12:27 刘歧  napisał(a):
> 
> 在 2017年8月28日，18:12，samsamsam  写道：
> 
> Validation will be very simple. I am talking about something like this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
> format_len = end - start - marker_len - 1 + strlen(PRId64);
> *o_format = av_mallocz(format_len+1);
> strncpy(*o_format, start + marker_len, end - start - marker_len -1);
> strcat(*o_format, PRId64);
> …
> }
> maybe more complex than this way, for example:
> %d
> %lld
> %04lld
> %PRId64
> %PRId32
> %PRId16
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> and so on,blablabla.
> 
> maybe we need to think a perfect solution.
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 11:30 Rodger Combs  napisał(a):
> 
> I would expect parsing the number internally and using the additional arg to 
> be simpler and easier to verify than format string validation.
> 
> On Aug 28, 2017, at 04:28, samsamsam  wrote:
> 
> OK. I will.
> What about adding validation of format instead of adding "something like 
> 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[刘歧]

> 在 2017年8月30日，15:52，samsamsam  写道：
> 
> What will happen when $Number%02s%08d$
> 
> Exacly. Such format is not allowed by dash specification. So, such validation 
> is sufficient.
> 
> Please check DASH specification format like that $Number%02s%08d$ is not 
> allowed. 
> So what for you want to allow it?
No, you have not get the point.
The hackers won’t refer to specification and just broke the rule to get root 
shell.

Anyway, merge your code and will update a new version patch later.

> 
> Dnia 30 sierpnia 2017 09:32 刘歧  napisał(a):
> 
> 在 2017年8月30日，14:38，samsamsam  写道：
> 
> But you do not added this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> What will happen when $Number%02s%08d$
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
>  format_len = end - start - marker_len - 1 + strlen(PRId64);
>  *o_format = av_mallocz(format_len+1);
>  strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>  strcat(*o_format, PRId64);
> …
> }
> 
> modification. Right?
> 
> Dnia 30 sierpnia 2017 00:04 Liu Steven  napisał(a):
> 
> 在 2017年8月30日，上午3:30，samsamsam  写道：
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> What? With my solution this is not problem.
> Why you think %s%d%d%d%d or %02c%lld give any problem?
> 
> localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
> ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg 
> developers
> built with Apple LLVM version 8.1.0 (clang-802.0.42)
> configuration: --enable-libass --enable-opengl --enable-libx264 
> --enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
> --enable-libopencv --enable-libtesseract --enable-libspeex 
> --enable-libfreetype --enable-libfontconfig --enable-libfdk-aac 
> --enable-videotoolbox --enable-libxml2
> libavutil  55. 74.100 / 55. 74.100
> libavcodec 57.103.101 / 57.103.101
> libavformat57. 78.100 / 57. 78.100
> libavdevice57.  7.101 / 57.  7.101
> libavfilter 6.100.100 /  6.100.100
> libswscale  4.  7.103 /  4.  7.103
> libswresample   2.  8.100 /  2.  8.100
> libpostproc54.  6.100 / 54.  6.100
> [dash @ 0x7ffb76802a00] 11
> Segmentation fault: 11ted 1 times
> localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
> /tmp/dash.mpd: initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1">
> localhost:dash StevenLiu$
> 
> reproduce:
> ffmpeg -i input -c copy -f dash /tmp/dash.mpd
> vim /tmp/dash.mpd
> seek to `media="chunk-stream$RepresentationID`
> the context is `  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="1”>`
> modify to:
>  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1”>
> 
> use your solution:
> ./ffmpeg -i /tmp/dash.mpd
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 12:27 刘歧  napisał(a):
> 
> 在 2017年8月28日，18:12，samsamsam  写道：
> 
> Validation will be very simple. I am talking about something like this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
>  format_len = end - start - marker_len - 1 + strlen(PRId64);
>  *o_format = av_mallocz(format_len+1);
>  strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>  strcat(*o_format, PRId64);
> …
> }
> maybe more complex than this way, for example:
> %d
> %lld
> %04lld
> %PRId64
> %PRId32
> %PRId16
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> and so on,blablabla.
> 
> maybe we need to think a perfect solution.
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 11:30 Rodger Combs  napisał(a):
> 
> I would expect parsing the number internally and using the additional arg to 
> be simpler and easier to verify than format string validation.
> 
> On Aug 28, 2017, at 04:28, samsamsam  wrote:
> 
> OK. I will.
> What about adding validation of format instead of adding "something like 
> `"%0*"PRId64`"?
> 
> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
> 
> If you know of such a vulnerability, report it to ffmpeg-secur...@ffmpeg.org. 
> New code with known vulnerabilities will not be 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[刘歧]

> 在 2017年8月30日，15:32，刘歧  写道：
> 
> 
>> 在 2017年8月30日，14:38，samsamsam  写道：
>> 
>> But you do not added this:
>> static int get_repl_pattern_and_format(const char *i_url, const char 
>> *i_marker, char **o_pattern, char **o_format)
>> {
>> ...
>> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
>> need to check this condition :P */
>> +if (*ptr != '0') {
> What will happen when $Number%02s%08d$ 

Test passed, now i have use a new function named get_segment_filename and the 
get_current_fragment,
that would check the file name ok or not.
>> +   // Unknown format add log here
>> +goto finish;
>> +}
>> +}
>>  format_len = end - start - marker_len - 1 + strlen(PRId64);
>>  *o_format = av_mallocz(format_len+1);
>>  strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>>  strcat(*o_format, PRId64);
>> …
>> }
>> 
>> modification. Right?
>> 
>> Dnia 30 sierpnia 2017 00:04 Liu Steven  napisał(a):
>> 
>> 在 2017年8月30日，上午3:30，samsamsam  写道：
>> 
>> and think about the safety :
>> %02c%lld
>> %s%d%d%d%d
>> 
>> What? With my solution this is not problem.
>> Why you think %s%d%d%d%d or %02c%lld give any problem?
>> 
>> localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
>> ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg 
>> developers
>> built with Apple LLVM version 8.1.0 (clang-802.0.42)
>> configuration: --enable-libass --enable-opengl --enable-libx264 
>> --enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
>> --enable-libopencv --enable-libtesseract --enable-libspeex 
>> --enable-libfreetype --enable-libfontconfig --enable-libfdk-aac 
>> --enable-videotoolbox --enable-libxml2
>> libavutil  55. 74.100 / 55. 74.100
>> libavcodec 57.103.101 / 57.103.101
>> libavformat57. 78.100 / 57. 78.100
>> libavdevice57.  7.101 / 57.  7.101
>> libavfilter 6.100.100 /  6.100.100
>> libswscale  4.  7.103 /  4.  7.103
>> libswresample   2.  8.100 /  2.  8.100
>> libpostproc54.  6.100 / 54.  6.100
>> [dash @ 0x7ffb76802a00] 11
>> Segmentation fault: 11ted 1 times
>> localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
>> /tmp/dash.mpd:   > initialization="init-stream$RepresentationID$.m4s" 
>> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1">
>> localhost:dash StevenLiu$
>> 
>> reproduce:
>> ffmpeg -i input -c copy -f dash /tmp/dash.mpd
>> vim /tmp/dash.mpd
>> seek to `media="chunk-stream$RepresentationID`
>> the context is ` > initialization="init-stream$RepresentationID$.m4s" 
>> media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="1”>`
>> modify to:
>> > initialization="init-stream$RepresentationID$.m4s" 
>> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1”>
>> 
>> use your solution:
>> ./ffmpeg -i /tmp/dash.mpd
>> 
>> 
>> 
>> 
>> Dnia 28 sierpnia 2017 12:27 刘歧  napisał(a):
>> 
>> 在 2017年8月28日，18:12，samsamsam  写道：
>> 
>> Validation will be very simple. I am talking about something like this:
>> static int get_repl_pattern_and_format(const char *i_url, const char 
>> *i_marker, char **o_pattern, char **o_format)
>> {
>> ...
>> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
>> need to check this condition :P */
>> +if (*ptr != '0') {
>> +   // Unknown format add log here
>> +goto finish;
>> +}
>> +}
>>  format_len = end - start - marker_len - 1 + strlen(PRId64);
>>  *o_format = av_mallocz(format_len+1);
>>  strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>>  strcat(*o_format, PRId64);
>> …
>> }
>> maybe more complex than this way, for example:
>> %d
>> %lld
>> %04lld
>> %PRId64
>> %PRId32
>> %PRId16
>> 
>> and think about the safety :
>> %02c%lld
>> %s%d%d%d%d
>> 
>> and so on,blablabla.
>> 
>> maybe we need to think a perfect solution.
>> 
>> 
>> 
>> 
>> Dnia 28 sierpnia 2017 11:30 Rodger Combs  napisał(a):
>> 
>> I would expect parsing the number internally and using the additional arg to 
>> be simpler and easier to verify than format string validation.
>> 
>> On Aug 28, 2017, at 04:28, samsamsam  wrote:
>> 
>> OK. I will.
>> What about adding validation of format instead of adding "something like 
>> `"%0*"PRId64`"?
>> 
>> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
>> 
>> If you know of such a vulnerability, report it to 
>> ffmpeg-secur...@ffmpeg.org. New code with known vulnerabilities will not be 
>> accepted.
>> 
>> Sent from my iPhone
>> 
>> On Aug 27, 2017, at 14:04, samsamsam  wrote:
>> get_repl_pattern_and_format, you should have a fixed value of something like 
>> `"%0*"PRId64`
>> 
>> If you afraid about safety then the only 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[刘歧]

> 在 2017年8月30日，14:38，samsamsam  写道：
> 
> But you do not added this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
What will happen when $Number%02s%08d$ 
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
>   format_len = end - start - marker_len - 1 + strlen(PRId64);
>   *o_format = av_mallocz(format_len+1);
>   strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>   strcat(*o_format, PRId64);
> …
> }
> 
> modification. Right?
> 
> Dnia 30 sierpnia 2017 00:04 Liu Steven  napisał(a):
> 
> 在 2017年8月30日，上午3:30，samsamsam  写道：
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> What? With my solution this is not problem.
> Why you think %s%d%d%d%d or %02c%lld give any problem?
> 
> localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
> ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg 
> developers
> built with Apple LLVM version 8.1.0 (clang-802.0.42)
> configuration: --enable-libass --enable-opengl --enable-libx264 
> --enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
> --enable-libopencv --enable-libtesseract --enable-libspeex 
> --enable-libfreetype --enable-libfontconfig --enable-libfdk-aac 
> --enable-videotoolbox --enable-libxml2
> libavutil  55. 74.100 / 55. 74.100
> libavcodec 57.103.101 / 57.103.101
> libavformat57. 78.100 / 57. 78.100
> libavdevice57.  7.101 / 57.  7.101
> libavfilter 6.100.100 /  6.100.100
> libswscale  4.  7.103 /  4.  7.103
> libswresample   2.  8.100 /  2.  8.100
> libpostproc54.  6.100 / 54.  6.100
> [dash @ 0x7ffb76802a00] 11
> Segmentation fault: 11ted 1 times
> localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
> /tmp/dash.mpd: initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1">
> localhost:dash StevenLiu$
> 
> reproduce:
> ffmpeg -i input -c copy -f dash /tmp/dash.mpd
> vim /tmp/dash.mpd
> seek to `media="chunk-stream$RepresentationID`
> the context is `  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%05d$.m4s" startNumber="1”>`
> modify to:
>  initialization="init-stream$RepresentationID$.m4s" 
> media="chunk-stream$RepresentationID$-$Number%s%05d$.m4s" startNumber="1”>
> 
> use your solution:
> ./ffmpeg -i /tmp/dash.mpd
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 12:27 刘歧  napisał(a):
> 
> 在 2017年8月28日，18:12，samsamsam  写道：
> 
> Validation will be very simple. I am talking about something like this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format)
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> +   // Unknown format add log here
> +goto finish;
> +}
> +}
>   format_len = end - start - marker_len - 1 + strlen(PRId64);
>   *o_format = av_mallocz(format_len+1);
>   strncpy(*o_format, start + marker_len, end - start - marker_len -1);
>   strcat(*o_format, PRId64);
> …
> }
> maybe more complex than this way, for example:
> %d
> %lld
> %04lld
> %PRId64
> %PRId32
> %PRId16
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> and so on,blablabla.
> 
> maybe we need to think a perfect solution.
> 
> 
> 
> 
> Dnia 28 sierpnia 2017 11:30 Rodger Combs  napisał(a):
> 
> I would expect parsing the number internally and using the additional arg to 
> be simpler and easier to verify than format string validation.
> 
> On Aug 28, 2017, at 04:28, samsamsam  wrote:
> 
> OK. I will.
> What about adding validation of format instead of adding "something like 
> `"%0*"PRId64`"?
> 
> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
> 
> If you know of such a vulnerability, report it to ffmpeg-secur...@ffmpeg.org. 
> New code with known vulnerabilities will not be accepted.
> 
> Sent from my iPhone
> 
> On Aug 27, 2017, at 14:04, samsamsam  wrote:
> get_repl_pattern_and_format, you should have a fixed value of something like 
> `"%0*"PRId64`
> 
> If you afraid about safety then the only thing which need to be added to 
> get_repl_pattern_and_format is validation of format.
> Simple loop to validate format will be enough. Do you agree?
> 
> Anyway we are talking about safety but parser for mp4 atoms missing checking 
> and there is quite easy to make segfault of the libavformat when try to 
> prepared mp4 file.
> 
> I 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[Liu Steven]

> 在 2017年8月30日，上午3:30，samsamsam  写道：
> 
> and think about the safety :
> %02c%lld
> %s%d%d%d%d
> 
> What? With my solution this is not problem.
> Why you think %s%d%d%d%d or %02c%lld give any problem?

localhost:dash StevenLiu$ ./ffmpeg -i /tmp/dash.mpd
ffmpeg version N-87079-g257f0d09f7 Copyright (c) 2000-2017 the FFmpeg developers
  built with Apple LLVM version 8.1.0 (clang-802.0.42)
  configuration: --enable-libass --enable-opengl --enable-libx264 
--enable-libmp3lame --enable-gpl --enable-nonfree --prefix=/usr/local 
--enable-libopencv --enable-libtesseract --enable-libspeex --enable-libfreetype 
--enable-libfontconfig --enable-libfdk-aac --enable-videotoolbox 
--enable-libxml2
  libavutil  55. 74.100 / 55. 74.100
  libavcodec 57.103.101 / 57.103.101
  libavformat57. 78.100 / 57. 78.100
  libavdevice57.  7.101 / 57.  7.101
  libavfilter 6.100.100 /  6.100.100
  libswscale  4.  7.103 /  4.  7.103
  libswresample   2.  8.100 /  2.  8.100
  libpostproc54.  6.100 / 54.  6.100
[dash @ 0x7ffb76802a00] 11
Segmentation fault: 11ted 1 times
localhost:dash StevenLiu$ grep -r "%s%05d" /tmp/dash.mpd
/tmp/dash.mpd:  
localhost:dash StevenLiu$

reproduce:
ffmpeg -i input -c copy -f dash /tmp/dash.mpd
vim /tmp/dash.mpd
seek to `media="chunk-stream$RepresentationID`
the context is `  `"%0*"PRId64`"?
> 
> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
> 
> If you know of such a vulnerability, report it to ffmpeg-secur...@ffmpeg.org. 
> New code with known vulnerabilities will not be accepted.
> 
> Sent from my iPhone
> 
> On Aug 27, 2017, at 14:04, samsamsam  wrote:
> get_repl_pattern_and_format, you should have a fixed value of something like 
> `"%0*"PRId64`
> 
> If you afraid about safety then the only thing which need to be added to 
> get_repl_pattern_and_format is validation of format.
> Simple loop to validate format will be enough. Do you agree?
> 
> Anyway we are talking about safety but parser for mp4 atoms missing checking 
> and there is quite easy to make segfault of the libavformat when try to 
> prepared mp4 file.
> 
> I understand that you want to have maximum safety with new code but I hope 
> you know that ffmpeg at all is not safety.
> 
> Regards,
> SSS
> 
> Dnia 27 sierpnia 2017 16:34 Rodger Combs  napisał(a):
> 
> You're still calling snprintf with a string derived from the XML, which is 
> still not safe. Rather than having a format copied from the source in 
> get_repl_pattern_and_format, you should have a fixed value of something like 
> `"%0*"PRId64`, and specify an additional "precision" argument you parse from 
> the XML yourself. I can't reiterate this enough: _never pass data from the 
> XML into the format-string arg of a printf-family function_.
> 
> Also, rather than calling snprintf() twice with an av_malloc() in between, 
> you can just call av_asprintf(). That's what it does internally anyway.
> 
> On Aug 27, 2017, at 09:19, Steven Liu  wrote:
> 
> ffmpeg need a dash demuxer for demux the dash formats base on
> https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch
> 
> TODO:
> 1. support multi bitrate dash
> 
> v2 fixed:
> 1. from autodetect to disabled
> 2. from camelCase code style to ffmpeg code style
> 3. from RepType to AVMediaType
> 4. fix variable typo
> 5. change time value from uint32_t to uint64_t
> 6. removed be used once API
> 7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and 
> av_timegm
> 8. merge complex free operation to free_fragment
> 9. use API from snprintf to av_asprintf
> 
> v3 fixed:
> 1. fix typo from --enabled-xml2 to --enable-xml2
> 
> v4 fixed:
> 1. from --enable-xml2 to --enable-libxml2
> 2. move system includes to top
> 3. remove nouse includes
> 4. rename enum name
> 5. add a trailing comma for the last entry enum
> 6. fix comment typo
> 7. add const to DASHContext class front
> 8. check sscanf if return arguments and give warning message when error
> 9. check validity before free seg->url and seg
> 10. check if the val is null, before use atoll
> 
> v5 fixed:
> 1. fix typo from mainifest to manifest
> 
> v6 fixed:
> 1. from realloc to av_realloc
> 2. from free to av_free
> 
> v7 fixed:
> 1. remove the -lxml2 from configure when require_pkg_config
> 
> v8 fixed:
> 1. fix replace filename template by av_asprintf secure problem
> 
> v9 modified:
> 1. make manifest parser clearly
> 
> v10 fixed:
> 1. fix function API name code style
> 2. remove redundant strreplace call
> 3. remove redundant memory operation and check return value from 
> get_content_url()
> 4. add space between ) and {
> 5. remove no need to log the value for print
> 
> v11 fixed:
> 1. from atoll to strtoll
> 
> v12 fixed:
> 1. remove strreplace and instead by av_strreplace
> 
> v13 fixed:
> 1. fix bug: cannot play:
> 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[wm4]

On Sun, 27 Aug 2017 22:19:31 +0800
Steven Liu  wrote:

> ffmpeg need a dash demuxer for demux the dash formats base on
> https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch
> 
> TODO:
> 1. support multi bitrate dash
> 
> v2 fixed:
> 1. from autodetect to disabled
> 2. from camelCase code style to ffmpeg code style
> 3. from RepType to AVMediaType
> 4. fix variable typo
> 5. change time value from uint32_t to uint64_t
> 6. removed be used once API
> 7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and 
> av_timegm
> 8. merge complex free operation to free_fragment
> 9. use API from snprintf to av_asprintf
> 
> v3 fixed:
> 1. fix typo from --enabled-xml2 to --enable-xml2
> 
> v4 fixed:
> 1. from --enable-xml2 to --enable-libxml2
> 2. move system includes to top
> 3. remove nouse includes
> 4. rename enum name
> 5. add a trailing comma for the last entry enum
> 6. fix comment typo
> 7. add const to DASHContext class front
> 8. check sscanf if return arguments and give warning message when error
> 9. check validity before free seg->url and seg
> 10. check if the val is null, before use atoll
> 
> v5 fixed:
> 1. fix typo from mainifest to manifest
> 
> v6 fixed:
> 1. from realloc to av_realloc
> 2. from free to av_free
> 
> v7 fixed:
> 1. remove the -lxml2 from configure when require_pkg_config
> 
> v8 fixed:
> 1. fix replace filename template by av_asprintf secure problem
> 
> v9 modified:
> 1. make manifest parser clearly
> 
> v10 fixed:
> 1. fix function API name code style
> 2. remove redundant strreplace call
> 3. remove redundant memory operation and check return value from 
> get_content_url()
> 4. add space between ) and {
> 5. remove no need to log the value for print
> 
> v11 fixed:
> 1. from atoll to strtoll
> 
> v12 fixed:
> 1. remove strreplace and instead by av_strreplace
> 
> v13 fixed:
> 1. fix bug: cannot play:
> http://dash.edgesuite.net/akamai/bbb_30fps/bbb_30fps.mpd
> 
> v14 fixed:
> 1. fix bug: TLS connection was non-properly terminated
> 2. fix bug: No trailing CRLF found in HTTP header
> 
> v15 fixed:
> 1. play youtube link: ffmpeg -i $(youtube-dl -J 
> "https://www.youtube.com/watch?v=XmL19DOP_Ls; | jq -r 
> ".requested_formats[0].manifest_url")
> 2. code refine for timeline living stream
> 
> Reviewed-by: Clément Bœsch 
> Reviewed-by: Michael Niedermayer 
> Reviewed-by: Carl Eugen Hoyos 
> Reviewed-by: Rodger Combs 
> Reviewed-by: Moritz Barsnick 
> Reviewed-by: Nicolas George 
> Reviewed-by: Ricardo Constantino 
> Reviewed-by: wm4 
> Tested-by: Andy Furniss 
> Reported-by: Andy Furniss 
> Signed-off-by: Steven Liu 
> Signed-off-by: samsamsam 

I only have some superficial comments, since I don't think I have a
full overview over the DASH protocol and this code to tell whether the
more intricate details are handled correctly. As such it's not a full
review.

But I'd like to object to the code duplication. Also, stuffing this
into a single source file isn't ideal.

Also, I don't remember having given a full review on this before.

> ---
>  configure|4 +
>  libavformat/Makefile |1 +
>  libavformat/allformats.c |2 +-
>  libavformat/dashdec.c| 1981 
> ++
>  4 files changed, 1987 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/dashdec.c
> 
> diff --git a/configure b/configure
> index 05f6dcc99a..7a7d61fa13 100755
> --- a/configure
> +++ b/configure
> @@ -272,6 +272,7 @@ External library support:
>--enable-libxcb-shapeenable X11 grabbing shape rendering [autodetect]
>--enable-libxvid enable Xvid encoding via xvidcore,
> native MPEG-4/Xvid encoder exists [no]
> +  --enable-libxml2 enable XML parsing using the C library libxml2 
> [no]
>--enable-libzimg enable z.lib, needed for zscale filter [no]
>--enable-libzmq  enable message passing via libzmq [no]
>--enable-libzvbi enable teletext support via libzvbi [no]
> @@ -1576,6 +1577,7 @@ EXTERNAL_LIBRARY_LIST="
>  libvpx
>  libwavpack
>  libwebp
> +libxml2
>  libzimg
>  libzmq
>  libzvbi
> @@ -2937,6 +2939,7 @@ avi_muxer_select="riffenc"
>  caf_demuxer_select="iso_media riffdec"
>  caf_muxer_select="iso_media"
>  dash_muxer_select="mp4_muxer"
> +dash_demuxer_deps="libxml2"
>  dirac_demuxer_select="dirac_parser"
>  dts_demuxer_select="dca_parser"
>  dtshd_demuxer_select="dca_parser"
> @@ -5996,6 +5999,7 @@ enabled openssl   && { use_pkg_config openssl 
> openssl/ssl.h OPENSSL_init
> check_lib openssl openssl/ssl.h 
> SSL_library_init -lssl -lcrypto -lws2_32 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[刘歧]

> 在 2017年8月28日，18:12，samsamsam  写道：
> 
> Validation will be very simple. I am talking about something like this:
> static int get_repl_pattern_and_format(const char *i_url, const char 
> *i_marker, char **o_pattern, char **o_format) 
> {
> ...
> +for(ptr=start + marker_len; ptr < (end - 1); ++ptr) {  /*there is 
> need to check this condition :P */
> +if (*ptr != '0') {
> +   // Unknown format add log here 
> +goto finish;
> +}
> +}
> format_len = end - start - marker_len - 1 + strlen(PRId64);
> *o_format = av_mallocz(format_len+1);
> strncpy(*o_format, start + marker_len, end - start - marker_len -1);
> strcat(*o_format, PRId64);
> …
> }
maybe more complex than this way, for example:
%d
%lld
%04lld
%PRId64
%PRId32
%PRId16

and think about the safety :
%02c%lld
%s%d%d%d%d

and so on,blablabla.

maybe we need to think a perfect solution.


> 
> 
> Dnia 28 sierpnia 2017 11:30 Rodger Combs  napisał(a):
> 
> I would expect parsing the number internally and using the additional arg to 
> be simpler and easier to verify than format string validation.
> 
>> On Aug 28, 2017, at 04:28, samsamsam  wrote:
>> 
>> OK. I will.
>> What about adding validation of format instead of adding "something like 
>> `"%0*"PRId64`"?
>> 
>> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
>> 
>> If you know of such a vulnerability, report it to 
>> ffmpeg-secur...@ffmpeg.org. New code with known vulnerabilities will not be 
>> accepted.
>> 
>> Sent from my iPhone
>> 
>> On Aug 27, 2017, at 14:04, samsamsam  wrote:
>>> get_repl_pattern_and_format, you should have a fixed value of something 
>>> like `"%0*"PRId64`
>>> 
>>> If you afraid about safety then the only thing which need to be added to 
>>> get_repl_pattern_and_format is validation of format.
>>> Simple loop to validate format will be enough. Do you agree? 
>>> 
>>> Anyway we are talking about safety but parser for mp4 atoms missing 
>>> checking and there is quite easy to make segfault of the libavformat when 
>>> try to prepared mp4 file.
>>> 
>>> I understand that you want to have maximum safety with new code but I hope 
>>> you know that ffmpeg at all is not safety.
>>> 
>>> Regards,
>>> SSS
>>> 
>>> Dnia 27 sierpnia 2017 16:34 Rodger Combs  
>>> napisał(a):
>>> 
>>> You're still calling snprintf with a string derived from the XML, which is 
>>> still not safe. Rather than having a format copied from the source in 
>>> get_repl_pattern_and_format, you should have a fixed value of something 
>>> like `"%0*"PRId64`, and specify an additional "precision" argument you 
>>> parse from the XML yourself. I can't reiterate this enough: _never pass 
>>> data from the XML into the format-string arg of a printf-family function_.
>>> 
>>> Also, rather than calling snprintf() twice with an av_malloc() in between, 
>>> you can just call av_asprintf(). That's what it does internally anyway.
>>> 
>>> On Aug 27, 2017, at 09:19, Steven Liu  wrote:
>>> 
>>> ffmpeg need a dash demuxer for demux the dash formats base on
>>> https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch
>>> 
>>> TODO:
>>> 1. support multi bitrate dash
>>> 
>>> v2 fixed:
>>> 1. from autodetect to disabled
>>> 2. from camelCase code style to ffmpeg code style
>>> 3. from RepType to AVMediaType
>>> 4. fix variable typo
>>> 5. change time value from uint32_t to uint64_t
>>> 6. removed be used once API
>>> 7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and 
>>> av_timegm
>>> 8. merge complex free operation to free_fragment
>>> 9. use API from snprintf to av_asprintf
>>> 
>>> v3 fixed:
>>> 1. fix typo from --enabled-xml2 to --enable-xml2
>>> 
>>> v4 fixed:
>>> 1. from --enable-xml2 to --enable-libxml2
>>> 2. move system includes to top
>>> 3. remove nouse includes
>>> 4. rename enum name
>>> 5. add a trailing comma for the last entry enum
>>> 6. fix comment typo
>>> 7. add const to DASHContext class front
>>> 8. check sscanf if return arguments and give warning message when error
>>> 9. check validity before free seg->url and seg
>>> 10. check if the val is null, before use atoll
>>> 
>>> v5 fixed:
>>> 1. fix typo from mainifest to manifest
>>> 
>>> v6 fixed:
>>> 1. from realloc to av_realloc
>>> 2. from free to av_free
>>> 
>>> v7 fixed:
>>> 1. remove the -lxml2 from configure when require_pkg_config
>>> 
>>> v8 fixed:
>>> 1. fix replace filename template by av_asprintf secure problem
>>> 
>>> v9 modified:
>>> 1. make manifest parser clearly
>>> 
>>> v10 fixed:
>>> 1. fix function API name code style
>>> 2. remove redundant strreplace call
>>> 3. remove redundant memory operation and check return value from 
>>> get_content_url()
>>> 4. add space between ) and {
>>> 5. remove no need to log the 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[Rodger Combs]

I would expect parsing the number internally and using the additional arg to be 
simpler and easier to verify than format string validation.

> On Aug 28, 2017, at 04:28, samsamsam  wrote:
> 
> OK. I will.
> What about adding validation of format instead of adding "something like 
> `"%0*"PRId64`"?
> 
> Dnia 28 sierpnia 2017 03:30 Rodger Combs  napisał(a):
> 
> If you know of such a vulnerability, report it to  
> ffmpeg-secur...@ffmpeg.org 
> . New code with known vulnerabilities will 
> not be accepted.
> 
> Sent from my iPhone
> 
> On Aug 27, 2017, at 14:04, samsamsam < 
> samsam...@o2.pl > wrote:
>> get_repl_pattern_and_format, you should have a fixed value of something like 
>> `"%0*"PRId64`
>> 
>> If you afraid about safety then the only thing which need to be added to 
>> get_repl_pattern_and_format is validation of format.
>> Simple loop to validate format will be enough. Do you agree? 
>> 
>> Anyway we are talking about safety but parser for mp4 atoms missing checking 
>> and there is quite easy to make segfault of the libavformat when try to 
>> prepared mp4 file.
>> 
>> I understand that you want to have maximum safety with new code but I hope 
>> you know that ffmpeg at all is not safety.
>> 
>> Regards,
>> SSS
>> 
>> Dnia 27 sierpnia 2017 16:34 Rodger Combs < 
>> rodger.co...@gmail.com 
>> > napisał(a):
>> 
>> You're still calling snprintf with a string derived from the XML, which is 
>> still not safe. Rather than having a format copied from the source in 
>> get_repl_pattern_and_format, you should have a fixed value of something like 
>> `"%0*"PRId64`, and specify an additional "precision" argument you parse from 
>> the XML yourself. I can't reiterate this enough: _never pass data from the 
>> XML into the format-string arg of a printf-family function_.
>> 
>> Also, rather than calling snprintf() twice with an av_malloc() in between, 
>> you can just call av_asprintf(). That's what it does internally anyway.
>> 
>> On Aug 27, 2017, at 09:19, Steven Liu < 
>> l...@chinaffmpeg.org 
>> > wrote:
>> 
>> ffmpeg need a dash demuxer for demux the dash formats base on
>> https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch
>>  
>> 
>> 
>> TODO:
>> 1. support multi bitrate dash
>> 
>> v2 fixed:
>> 1. from autodetect to disabled
>> 2. from camelCase code style to ffmpeg code style
>> 3. from RepType to AVMediaType
>> 4. fix variable typo
>> 5. change time value from uint32_t to uint64_t
>> 6. removed be used once API
>> 7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and 
>> av_timegm
>> 8. merge complex free operation to free_fragment
>> 9. use API from snprintf to av_asprintf
>> 
>> v3 fixed:
>> 1. fix typo from --enabled-xml2 to --enable-xml2
>> 
>> v4 fixed:
>> 1. from --enable-xml2 to --enable-libxml2
>> 2. move system includes to top
>> 3. remove nouse includes
>> 4. rename enum name
>> 5. add a trailing comma for the last entry enum
>> 6. fix comment typo
>> 7. add const to DASHContext class front
>> 8. check sscanf if return arguments and give warning message when error
>> 9. check validity before free seg->url and seg
>> 10. check if the val is null, before use atoll
>> 
>> v5 fixed:
>> 1. fix typo from mainifest to manifest
>> 
>> v6 fixed:
>> 1. from realloc to av_realloc
>> 2. from free to av_free
>> 
>> v7 fixed:
>> 1. remove the -lxml2 from configure when require_pkg_config
>> 
>> v8 fixed:
>> 1. fix replace filename template by av_asprintf secure problem
>> 
>> v9 modified:
>> 1. make manifest parser clearly
>> 
>> v10 fixed:
>> 1. fix function API name code style
>> 2. remove redundant strreplace call
>> 3. remove redundant memory operation and check return value from 
>> get_content_url()
>> 4. add space between ) and {
>> 5. remove no need to log the value for print
>> 
>> v11 fixed:
>> 1. from atoll to strtoll
>> 
>> v12 fixed:
>> 1. remove strreplace and instead by av_strreplace
>> 
>> v13 fixed:
>> 1. fix bug: cannot play:
>> http://dash.edgesuite.net/akamai/bbb_30fps/bbb_30fps.mpd 
>> 
>> 
>> v14 fixed:
>> 1. fix bug: TLS connection was non-properly terminated
>> 2. fix bug: No trailing CRLF found in HTTP header
>> 
>> v15 fixed:
>> 1. play youtube link: ffmpeg -i $(youtube-dl -J 
>> "https://www.youtube.com/watch?v=XmL19DOP_Ls; 
>>  | jq -r 
>> ".requested_formats[0].manifest_url")
>> 2. code refine for timeline living stream
>> 
>> Reviewed-by: Clément Bœsch < u...@pkh.me 
>> 

Re: [FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[Rodger Combs]

You're still calling snprintf with a string derived from the XML, which is 
still not safe. Rather than having a format copied from the source in 
get_repl_pattern_and_format, you should have a fixed value of something like 
`"%0*"PRId64`, and specify an additional "precision" argument you parse from 
the XML yourself. I can't reiterate this enough: _never pass data from the XML 
into the format-string arg of a printf-family function_.

Also, rather than calling snprintf() twice with an av_malloc() in between, you 
can just call av_asprintf(). That's what it does internally anyway.

> On Aug 27, 2017, at 09:19, Steven Liu  wrote:
> 
> ffmpeg need a dash demuxer for demux the dash formats base on
> https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch
> 
> TODO:
> 1. support multi bitrate dash
> 
> v2 fixed:
> 1. from autodetect to disabled
> 2. from camelCase code style to ffmpeg code style
> 3. from RepType to AVMediaType
> 4. fix variable typo
> 5. change time value from uint32_t to uint64_t
> 6. removed be used once API
> 7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and 
> av_timegm
> 8. merge complex free operation to free_fragment
> 9. use API from snprintf to av_asprintf
> 
> v3 fixed:
> 1. fix typo from --enabled-xml2 to --enable-xml2
> 
> v4 fixed:
> 1. from --enable-xml2 to --enable-libxml2
> 2. move system includes to top
> 3. remove nouse includes
> 4. rename enum name
> 5. add a trailing comma for the last entry enum
> 6. fix comment typo
> 7. add const to DASHContext class front
> 8. check sscanf if return arguments and give warning message when error
> 9. check validity before free seg->url and seg
> 10. check if the val is null, before use atoll
> 
> v5 fixed:
> 1. fix typo from mainifest to manifest
> 
> v6 fixed:
> 1. from realloc to av_realloc
> 2. from free to av_free
> 
> v7 fixed:
> 1. remove the -lxml2 from configure when require_pkg_config
> 
> v8 fixed:
> 1. fix replace filename template by av_asprintf secure problem
> 
> v9 modified:
> 1. make manifest parser clearly
> 
> v10 fixed:
> 1. fix function API name code style
> 2. remove redundant strreplace call
> 3. remove redundant memory operation and check return value from 
> get_content_url()
> 4. add space between ) and {
> 5. remove no need to log the value for print
> 
> v11 fixed:
> 1. from atoll to strtoll
> 
> v12 fixed:
> 1. remove strreplace and instead by av_strreplace
> 
> v13 fixed:
> 1. fix bug: cannot play:
> http://dash.edgesuite.net/akamai/bbb_30fps/bbb_30fps.mpd
> 
> v14 fixed:
> 1. fix bug: TLS connection was non-properly terminated
> 2. fix bug: No trailing CRLF found in HTTP header
> 
> v15 fixed:
> 1. play youtube link: ffmpeg -i $(youtube-dl -J 
> "https://www.youtube.com/watch?v=XmL19DOP_Ls; | jq -r 
> ".requested_formats[0].manifest_url")
> 2. code refine for timeline living stream
> 
> Reviewed-by: Clément Bœsch 
> Reviewed-by: Michael Niedermayer 
> Reviewed-by: Carl Eugen Hoyos 
> Reviewed-by: Rodger Combs 
> Reviewed-by: Moritz Barsnick 
> Reviewed-by: Nicolas George 
> Reviewed-by: Ricardo Constantino 
> Reviewed-by: wm4 
> Tested-by: Andy Furniss 
> Reported-by: Andy Furniss 
> Signed-off-by: Steven Liu 
> Signed-off-by: samsamsam 
> ---
> configure|4 +
> libavformat/Makefile |1 +
> libavformat/allformats.c |2 +-
> libavformat/dashdec.c| 1981 ++
> 4 files changed, 1987 insertions(+), 1 deletion(-)
> create mode 100644 libavformat/dashdec.c
> 
> diff --git a/configure b/configure
> index 05f6dcc99a..7a7d61fa13 100755
> --- a/configure
> +++ b/configure
> @@ -272,6 +272,7 @@ External library support:
>   --enable-libxcb-shapeenable X11 grabbing shape rendering [autodetect]
>   --enable-libxvid enable Xvid encoding via xvidcore,
>native MPEG-4/Xvid encoder exists [no]
> +  --enable-libxml2 enable XML parsing using the C library libxml2 
> [no]
>   --enable-libzimg enable z.lib, needed for zscale filter [no]
>   --enable-libzmq  enable message passing via libzmq [no]
>   --enable-libzvbi enable teletext support via libzvbi [no]
> @@ -1576,6 +1577,7 @@ EXTERNAL_LIBRARY_LIST="
> libvpx
> libwavpack
> libwebp
> +libxml2
> libzimg
> libzmq
> libzvbi
> @@ -2937,6 +2939,7 @@ avi_muxer_select="riffenc"
> caf_demuxer_select="iso_media riffdec"
> caf_muxer_select="iso_media"
> dash_muxer_select="mp4_muxer"
> +dash_demuxer_deps="libxml2"
> dirac_demuxer_select="dirac_parser"
> dts_demuxer_select="dca_parser"
> dtshd_demuxer_select="dca_parser"
> @@ -5996,6 +5999,7 @@ enabled openssl   && { 

[FFmpeg-devel] [PATCH v15] avformat/dashdec: add dash demuxer base version

[Steven Liu]

ffmpeg need a dash demuxer for demux the dash formats base on
https://github.com/samsamsam-iptvplayer/exteplayer3/blob/master/tmp/ffmpeg/patches/3.2.2/01_add_dash_demux.patch

TODO:
1. support multi bitrate dash

v2 fixed:
1. from autodetect to disabled
2. from camelCase code style to ffmpeg code style
3. from RepType to AVMediaType
4. fix variable typo
5. change time value from uint32_t to uint64_t
6. removed be used once API
7. change 'time(NULL)`, except it is not 2038-safe.' to av_gettime and av_timegm
8. merge complex free operation to free_fragment
9. use API from snprintf to av_asprintf

v3 fixed:
1. fix typo from --enabled-xml2 to --enable-xml2

v4 fixed:
1. from --enable-xml2 to --enable-libxml2
2. move system includes to top
3. remove nouse includes
4. rename enum name
5. add a trailing comma for the last entry enum
6. fix comment typo
7. add const to DASHContext class front
8. check sscanf if return arguments and give warning message when error
9. check validity before free seg->url and seg
10. check if the val is null, before use atoll

v5 fixed:
1. fix typo from mainifest to manifest

v6 fixed:
1. from realloc to av_realloc
2. from free to av_free

v7 fixed:
1. remove the -lxml2 from configure when require_pkg_config

v8 fixed:
1. fix replace filename template by av_asprintf secure problem

v9 modified:
1. make manifest parser clearly

v10 fixed:
1. fix function API name code style
2. remove redundant strreplace call
3. remove redundant memory operation and check return value from 
get_content_url()
4. add space between ) and {
5. remove no need to log the value for print

v11 fixed:
1. from atoll to strtoll

v12 fixed:
1. remove strreplace and instead by av_strreplace

v13 fixed:
1. fix bug: cannot play:
http://dash.edgesuite.net/akamai/bbb_30fps/bbb_30fps.mpd

v14 fixed:
1. fix bug: TLS connection was non-properly terminated
2. fix bug: No trailing CRLF found in HTTP header

v15 fixed:
1. play youtube link: ffmpeg -i $(youtube-dl -J 
"https://www.youtube.com/watch?v=XmL19DOP_Ls; | jq -r 
".requested_formats[0].manifest_url")
2. code refine for timeline living stream

Reviewed-by: Clément Bœsch 
Reviewed-by: Michael Niedermayer 
Reviewed-by: Carl Eugen Hoyos 
Reviewed-by: Rodger Combs 
Reviewed-by: Moritz Barsnick 
Reviewed-by: Nicolas George 
Reviewed-by: Ricardo Constantino 
Reviewed-by: wm4 
Tested-by: Andy Furniss 
Reported-by: Andy Furniss 
Signed-off-by: Steven Liu 
Signed-off-by: samsamsam 
---
 configure|4 +
 libavformat/Makefile |1 +
 libavformat/allformats.c |2 +-
 libavformat/dashdec.c| 1981 ++
 4 files changed, 1987 insertions(+), 1 deletion(-)
 create mode 100644 libavformat/dashdec.c

diff --git a/configure b/configure
index 05f6dcc99a..7a7d61fa13 100755
--- a/configure
+++ b/configure
@@ -272,6 +272,7 @@ External library support:
   --enable-libxcb-shapeenable X11 grabbing shape rendering [autodetect]
   --enable-libxvid enable Xvid encoding via xvidcore,
native MPEG-4/Xvid encoder exists [no]
+  --enable-libxml2 enable XML parsing using the C library libxml2 [no]
   --enable-libzimg enable z.lib, needed for zscale filter [no]
   --enable-libzmq  enable message passing via libzmq [no]
   --enable-libzvbi enable teletext support via libzvbi [no]
@@ -1576,6 +1577,7 @@ EXTERNAL_LIBRARY_LIST="
 libvpx
 libwavpack
 libwebp
+libxml2
 libzimg
 libzmq
 libzvbi
@@ -2937,6 +2939,7 @@ avi_muxer_select="riffenc"
 caf_demuxer_select="iso_media riffdec"
 caf_muxer_select="iso_media"
 dash_muxer_select="mp4_muxer"
+dash_demuxer_deps="libxml2"
 dirac_demuxer_select="dirac_parser"
 dts_demuxer_select="dca_parser"
 dtshd_demuxer_select="dca_parser"
@@ -5996,6 +5999,7 @@ enabled openssl   && { use_pkg_config openssl 
openssl/ssl.h OPENSSL_init
check_lib openssl openssl/ssl.h 
SSL_library_init -lssl -lcrypto -lws2_32 -lgdi32 ||
die "ERROR: openssl not found"; }
 enabled qtkit_indev  && { check_header_objcc QTKit/QTKit.h || disable 
qtkit_indev; }
+enabled libxml2  && require_pkg_config libxml-2.0 
libxml2/libxml/xmlversion.h xmlCheckVersion
 
 if enabled gcrypt; then
 GCRYPT_CONFIG="${cross_prefix}libgcrypt-config"
diff --git a/libavformat/Makefile b/libavformat/Makefile
index f2b465cfa2..3d478749d0 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -133,6 +133,7 @@ OBJS-$(CONFIG_CRC_MUXER) += crcenc.o
 OBJS-$(CONFIG_DATA_DEMUXER)  += rawdec.o
 OBJS-$(CONFIG_DATA_MUXER)+= rawenc.o
 OBJS-$(CONFIG_DASH_MUXER)+= dashenc.o