Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ICAGeg has it difficult to process Multipart/form-data containing multiple uploaded files #155

Open
idavollen opened this issue Jan 26, 2024 · 2 comments

Comments

@idavollen
Copy link

idavollen commented Jan 26, 2024

If the multipart/form-data post request contains multiple file uploads, our WAF failed to report the file upload containing the EIcar testing attack signature under some scenarios:

when the concerned malicious file upload is NOT the first Content-Disposition


POST /fileupload/ HTTP/1.1
Host: my.hostname.com
Content-Length: 906
Sec-Ch-Ua: "Not_A Brand";v="8", "Chromium";v="120"
Content-Type: multipart/form-data; boundary=----WebKitFormBoundaryroEHrYoGcEwB3fY5

------WebKitFormBoundaryroEHrYoGcEwB3fY5
Content-Disposition: form-data; name="dataService"; filename="blob"
Content-Type: application/json

{"delivery":"EMAIL","exampleReportName":null,"frequency":"WEEKLY","hasExampleReport":false,"hasProductSheet":false,"hasTermsFile":false,"hasImage":false,"id":null,"identification":null,"priceRange1":"45","priceRange2":"454","priceRange3":"454","priceRange4":"445","productInformation":"asdf","productName":"fas","productSheetFileName":"","imageFileName":"image-2023-01-30-08-00-51-575.png","termsFileName":"","types":["ALL_ISINS"],"status":"NOT_ACTIVE","isins":[],"hasImageFile":true}
------WebKitFormBoundaryroEHrYoGcEwB3fY5
Content-Disposition: form-data; name="image"; filename="image-2023-01-30-08-00-51-575.png"
Content-Type: image/png

X5O!P%@AP[4\PZX54(P^)7CC)7}$EICAR-STANDARD-ANTIVIRUS-TEST-FILE!$H+H*
------WebKitFormBoundaryroEHrYoGcEwB3fY5--

However, if I put the same PNG file upload containing the EICar attack signature in the first Content-Disposition or I manually changed filename="blob" into filename123="blob" with the Burp Suite tools, our WAF can successfully block this malicious file upload with Multipart/form-data. I had thought that our WAF has problem to handle the multipart/form-data. All of sudden, I realized that I'm wrong, because our WAF as an ICAP client only encapsulates the income multipart/form-data as a REQMOD message and sends it to the ICAP server who has responsibilities to parse the mulipart/form-data post data, extracting the uploaded files and ask its associated AV scanning service to scan these uploaded files.

After code analyzing your implementation https://github.com/egirna/icapeg/blob/master/service/services-utilities/ContentTypes/multipartForm.go#L45-52, I think I've found the explanations for our observed problems #153


        var theFile FormPart
	for i := 0; i < len(formParts); i++ {
		if formParts[i].FileName != "" {
			theFile = formParts[i]
			break
		}
	}
	return formParts, theFile, boundary

As the above code snippet shows, your implementation is only interested in the firstly found filename specified in the Content-Disposition, which fits well with our observations.

How do you think of collecting all file uploads in the multipart/form-data rather than break after the first matched filename, and then looping them at https://github.com/egirna/icapeg/blob/master/service/services-utilities/ContentTypes/contentType.go#L24 ?

If you think it makes sense, refactoring your implementation in time will be really appreciated!

@idavollen
Copy link
Author

@mahnouman any comments or feedbacks?

@idavollen
Copy link
Author

It seems that the clamav.go tries to scan all uploaded files. Pls correct me if I had thought it wrong

// no need to scan part of the file, this service needs all the file at one time
	if partial {
		logging.Logger.Info(utils.PrepareLogMsg(c.xICAPMetadata,
			c.serviceName+" service has stopped processing partially"))
		return utils.Continue, nil, nil,
			msgHeadersBeforeProcessing, msgHeadersAfterProcessing, vendorMsgs
	}

I've tried to change


// GetFileFromRequest is used for parsing the multipart form
func (m MultipartForm) GetFileFromRequest() *bytes.Buffer {
	return bytes.NewBuffer(m.theFile.Content)
}

into


func (m *MultipartForm) GetFileFromRequest() *bytes.Buffer {
	return bytes.NewBufferString(m.creatingMultipartForm())
}

but it doesn't work as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant