Open idavollen opened 10 months ago
@mahnouman any comments or feedbacks?
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
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
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
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!