all lists on 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal