[FFmpeg-devel] [PATCH] [fateserver] Cleanup and security strengthening

[Nicolas George]

Chad Fraleigh (12021-08-22):
> It mostly looks good (from a perl perspective).

Thanks for the comments.

> 
> Just 3 questionable items..
> 
> -<>-<>-
> 
> -if ($ENV{HTTP_ACCEPT_ENCODING} =~ /gzip/) {
> -    print "Content-Encoding: gzip\r\n";
> +if (ready_for_gzip) {
>      $cat = 'cat';
>  }
> 
> The old code outputs "\r\n", where ready_for_gzip() outputs "\r\n\r\n". Will
> this be an issue, or should it have done that in the first place?

There is a “-print "\r\n";” just a little below, that balances out.

> 
> -<>-<>-
> 
> +sub ready_for_gzip() {
> +    my $ae = $ENV{HTTP_ACCEPT_ENCODING};
> +    if (defined($ae) && $ae =~ /gzip/) {
> 
> It is checking for 'gzip' as a substring, rather than a whole word. If it
> was passed a [hypothetical] encoding type contains the substring gzip (e.g.
> "bigzip"), it could trigger in incompatible output encoding. However, it's
> not any worse than it was previously.
> 
> Perhaps changing it to match /\bgzip\b/ ?

I wanted minimalistic changes, but this is better.

> 
> -<>-<>-
> 
>  sub ready_for_gzip() {
> +    # Under CGI, $PATH is safe
> +    ($ENV{PATH}) = $ENV{PATH} =~ /(.*)/s;
> 
> It is untainting the PATH as "hidden" side effect of calling
> ready_for_gzip(). While technically it works, it feels a little kludgy
> compared to untainting it at the beginning of each taint-enabled script.

You are right. Changed.

I will try to deploy the changes this shortly.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-FATE-cosmetic-reorder-EXPORT.patch
Type: text/x-diff
Size: 1004 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-.cgi-hardcode-Perl-library-path.patch
Type: text/x-diff
Size: 1403 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-index-remove-uri.patch
Type: text/x-diff
Size: 1853 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-FATE-add-functions-to-validate-parameters.patch
Type: text/x-diff
Size: 1774 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-all-uniformize-compression-handling.patch
Type: text/x-diff
Size: 3331 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-history-validate-parameters.patch
Type: text/x-diff
Size: 710 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0005.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0007-report-validate-parameters.patch
Type: text/x-diff
Size: 798 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0006.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0008-log-validate-parameters.patch
Type: text/x-diff
Size: 1017 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0007.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0009-index-validate-parameters.patch
Type: text/x-diff
Size: 1039 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0008.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0010-.cgi-enable-taint-checks.patch
Type: text/x-diff
Size: 2808 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0009.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0011-index-force-scalar-context-for-param-query.patch
Type: text/x-diff
Size: 1002 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment-0010.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20210823/5535f3be/attachment.sig>