public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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!




      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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal