From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Friedrich Weber <f.weber@proxmox.com>
Subject: [pve-devel] applied: [PATCH http-server] multipart upload: properly parse file parts without Content-Type
Date: Tue, 11 Apr 2023 14:43:46 +0200 [thread overview]
Message-ID: <676c2cee-5e3c-7195-e848-3eae16e47f7b@proxmox.com> (raw)
In-Reply-To: <20230411121947.124433-1-f.weber@proxmox.com>
Am 11/04/2023 um 14:19 schrieb Friedrich Weber:
> 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: 3e3faddb4a3792557351f1a6e9f2685e4713eb24
> Link: https://forum.proxmox.com/threads/125411/
> Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
> ---
> src/PVE/APIServer/AnyEvent.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>
applied, thanks!
prev parent reply other threads:[~2023-04-11 12:44 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-11 12:19 [pve-devel] " Friedrich Weber
2023-04-11 12:43 ` Thomas Lamprecht [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=676c2cee-5e3c-7195-e848-3eae16e47f7b@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=f.weber@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox