mirror of
https://git.proxmox.com/git/pve-http-server
synced 2025-05-29 18:45:31 +00:00
multipart upload: properly parse file parts without Content-Type
As reported in the forum, multipart requests are parsed incorrectly if
the file part header contains *only* Content-Disposition, but no other
fields (in particular, no Content-Type). As a result, uploaded files
are mangled: In most cases, an additional carriage return and line
feed (\r\n) is prepended to the file contents.
As an example, consider the following file part (with explicit \r\n
for clarity):
Content-Disposition: form-data; name=...; filename=...\r\n
Content-Type: application/x-iso9660-image\r\n
\r\n
file contents...
The current parsing code for file parts roughly works as follows:
1) Consume the Content-Disposition field including the trailing \r\n
2) Consume and ignore everything up to and including the next \r\n\r\n
3) Read the file contents
This works fine in the example above. However, it has a bug in case
Content-Disposition is the *only* header field:
Content-Disposition: form-data; name=...; filename=...\r\n
\r\n
file contents...
Now, step 1 already consumes the first half of the \r\n\r\n sequence
that marks the end of the part headers. As a result, step 3 starts
reading the file at a wrong offset:
- If the remaining contents of the read buffer (currently sized 16KiB)
contain \r\n\r\n, step 2 consumes everything up to and including
this marker and step 3 starts reading file contents there. As a
result, the uploaded file is truncated at its beginning.
- Otherwise, step 2 is a noop and step 3 considers the remaining
second half of the \r\n\r\n marker to be part of the file contents.
As a result, the uploaded file is prepended with an extra \r\n.
To fix this, modify step 1 to *not* consume the trailing \r\n. This
keeps the \r\n\r\n marker intact, no matter whether additional header
fields are present or not.
Fixes: 3e3faddb4a
Link: https://forum.proxmox.com/threads/125411/
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
This commit is contained in:
parent
fcb543a682
commit
602eb8aabd
@ -1222,7 +1222,7 @@ sub file_upload_multipart {
|
||||
$extract_form_disposition->('checksum-algorithm');
|
||||
$extract_form_disposition->('checksum');
|
||||
|
||||
if ($hdl->{rbuf} =~ s/^${delim_re}Content-Disposition: (.*?); name="(.*?)"; filename="([^"]+)"${newline_re}//s) {
|
||||
if ($hdl->{rbuf} =~ s/^${delim_re}Content-Disposition: (.*?); name="(.*?)"; filename="([^"]+)"//s) {
|
||||
assert_form_disposition($1);
|
||||
die "wrong field name '$2' for file upload, expected 'filename'" if $2 ne "filename";
|
||||
$rstate->{phase} = 2;
|
||||
|
Loading…
Reference in New Issue
Block a user