| Subject: | HTTP::Body::Multipart critical security bug |
| Date: | Mon, 02 Sep 2013 17:39:04 +0200 |
| To: | bug-HTTP-Body [...] rt.cpan.org |
| From: | Jonathan Dollé <jonathan.dolle [...] groupsquad.com> |
Hello,
We discovered a critical bug in HTTP::Body::Multipart >= 1.08.
It concerns this point :
Temp files now preserve the suffix of the uploaded file. This makes
it possible to feed the file directly into a mime-type-determining
module that may rely on this suffix as part of its heuristic. (Dave
Rolsky)
The following line is not good :
my $suffix = $basename =~ /[^.]+(\.[^\\\/]+)$/ ? $1 : q{};
It is too much permissive.
For example, with the following file name :
"2013-06-19 at 11.37.56 PM.png"
We can obtain this temp file :
"/tmp/k6gvivOIYK.37.56 PM.png"
It take everithing after the first dot, even spaces !
Previously, the tempname was always alphanumeric. No special chars. So
we could use it directly in commands like:
my $info = `identify -format "%m" $filename 2>&1`;
With a space, the command become invalid. Worse : we can easily do
'injections'.
For example with a filename like:
"file. || rm -rf ~ || .png"
I recommand the following regexp:
my $suffix = $basename =~ /[^.]+(\.[\w]+)$/ ? $1 : q{};
Or, for extension like '.tar.gz':
my $suffix = $basename =~ /[^.]+(\.[\w\.]+)$/ ? $1 : q{};
Or better:
my $suffix = $basename =~ /[^.]+((?:\.[\w+])+)$/ ? $1 : q{};
Best regards,
Jonathan Dolle