Multipart_Message.Store_Attachments allows files to be uploaded on the server even is Upload_Directory is disabled (empty String)

[dsauvage]

Multipart_Message.Store_Attachments allows files to be uploaded on the server even is Upload_Directory is disabled (empty String)

To fix this issue we applied the same verification and error management as Multipart_Message.File_Upload (patch attached).

--- aws-server-http_utils.adb.orig	2024-04-13 11:49:53.691588811 +0400
+++ aws-server-http_utils.adb	2024-04-13 11:50:55.539068778 +0400
@@ -857,6 +857,11 @@
       begin
          begin
             if Mode in Attachment .. File_Upload then
+               if CNF.Upload_Directory (Server_Config) = "" then
+                  raise Constraint_Error
+                    with "File upload not supported by server "
+                      & CNF.Server_Name (Server_Config);
+               end if;
                Streams.Stream_IO.Create
                  (File, Streams.Stream_IO.Out_File, Server_Filename);
             end if;

Reproducer file command.sh attached, request payload below;

POST / HTTP/1.1
Host: localhost:8080
User-Agent: curl/7.74.0
Accept: */*
Content-Length: 500
Content-Type: multipart/related; boundary=------------------------e3b8d4247741cbe4

--------------------------e3b8d4247741cbe4
Content-Disposition: attachment; name="file";filename="threat"
Content-Type: application/octet-stream
Content-Id: dude

THREAT AGENT

--------------------------e3b8d4247741cbe4--

In this case, as the Content-Length is bigger than the actual payload, the web server is waiting and the temporary uploaded file is not yet deleted. A simple ls command executed in the directory where the web server has been launched will show the temporary file.

$ ls
27495-1

Another way to assess the temporary uploaded file is by using the inotifywait command executed in the directory where the web server has been launched

$ inotifywait -m .
./ CREATE 27495-1
./ OPEN 27495-1
./ MODIFY 27495-1
./ CLOSE_WRITE,CLOSE 27495-1

aws-server-http_utils.adb.changes.patch.txt

command.sh.txt

[TurboGit]

@dsauvage : This patch looks wrong to me, I suppose we want the exception only if Mode = File_Upload. When there is an attachment (MIME multipart) we certainly don't want to fail if Upload_Directory is not set.

[dsauvage]

Well it seems I does not understand your rational.

• A POST request with Content-Type "multipart/form-data" potentially leads to a file been written on the server [1].
• A POST request with Content-Type "multipart/related" potentially leads attachments to be written to separate files [2]

If file uploads are not supported by server, both entry points above should be tackled, and not only the first one.

[1] aws-server-http_utils.adb:1238
[2] aws-server-http_utils.adb:1250

[TurboGit]

@dsauvage : Well both cases are different.

multipart/form-data is used to upload files of MIME-compatible representation, such as pictures and video files, and related metadata a single POST request.

multipart/related is used for compound documents and you would need to combine the separate body parts to provide the full meaning of the message.

Only the first one is covered by "file upload" in AWS as it is really an explicit file upload.

The multipart/related is just a message containing different related parts and at the AWS user's point of view we don't want them to force allowing file upload feature.

Hope this clarifies the rational.

[dsauvage]

1. When multipart/related and multipart/form-data POST requests are processed, having Upload_Directory set. The requests files are written in the Upload_Directory folder. So the Upload_Directory parameter is associated to both request content types.
2. When a multipart/form-data POST request is processed, having Upload_Directory unset, an HTTP status 500 is returned, mentioning "File upload not supported by server"
3. However, when a multipart/related POST request is processed, having Upload_Directory unset, the files are written in the folder on the application executable, which is incoherent, unsafe and erroneous

Hope you will reconsider your assessment of this issue.

[TurboGit]

Hope you will reconsider your assessment of this issue.

I'm not sure what you're expecting. But raising an exception here is wrong as uploading is only a multipart/form-data feature. If we raise an exception now we will break all existing servers which is not an option.

I'm not saying there is no issue and we certainly can improve this, but the patch here is not the way to go.