0

I have asp.net mvc application which has file upload functionality. While uploading the file, I am performing few validations on the uploaded content before moving it to database and file system location.

Here goes my code:

[HttpPost]
[ValidateAntiForgeryToken]
public ActionResult AddImage([Bind(Include = "image,ImageName,ImageType,CountryId,Keyword,Source,Copyright,Description")] CreateImageViewModel model)
{
    if (!this.ModelState.IsValid)
    {
        return View("Images");
    }

    if (model != null && model.image.ContentType.Contains(Constants.Image) && !ValidateUploadedImageContent(model.image, model.image.FileName))
    {
        var dto = new ImageDTO();
        model.FilePath = model.image.FileName;
        dto.ImageFile = model.image;
        dto.Name = model.ImageName;
        dto.FilePath = model.image.FileName;
        dto.FileType = Path.GetExtension(model.FilePath);
        dto.ImageType = model.ImageType;
        dto.CountryId = model.CountryId;
        dto.Keyword = model.Keyword;
        dto.Source = model.Source;
        dto.Copyright = model.Copyright;
        dto.Description = model.Description;
        dto.CreatedBy = UserDto.emailId;
        try
        {
            _imageService.SaveImage(dto);
        }
        catch (Exception ex)
        {
            if (ex.Message.Equals(Constants.InvalidImageType))
                return GetSafeRedirect(Url.Action("AddImage", model) + "#onload-errors");
            throw ex;
        }

        return RedirectToAction(Constants.Actions.Images.ToString());
    }
    else
    {
        return GetSafeRedirect(Url.Action("AddImage", model) + "#onload-errors");
    }
}

private bool ValidateUploadedImageContent(HttpPostedFileBase uploadedFile, string imageFileName)
{
    if (Path.GetExtension(imageFileName).Equals(".svg", StringComparison.OrdinalIgnoreCase))
    {
        if (uploadedFile.ContentLength > 0)
        {
            byte[] data;
            //using (Stream inputStream = uploadedFile.InputStream)
            //{
            Stream inputStream = uploadedFile.InputStream;
            var memoryStream = inputStream as MemoryStream;
            if (memoryStream == null)
            {
                memoryStream = new MemoryStream();
                inputStream.CopyTo(memoryStream);
            }

            data = memoryStream.ToArray();
            //}
            var parsedData = Encoding.UTF8.GetString(data, 0, data.Length).TrimEnd('\0');
            var result = parsedData.ContainsAny(Constants.InsecureStrings, StringComparison.CurrentCultureIgnoreCase);
            return result;
        }
    }

    return false;
}

Here in the above method: ValidateUploadedImageContent(), I tried to dispose the stream object with the help of using statement but I found that if I keep the below code in the method: ValidateUploadedImageContent(), then in that case post validation process, I found on debugging that the ContentLength property is set with 0 value and finally corrupted image gets saved in the file system location.

Updated :

private bool ValidateUploadedImageContent(HttpPostedFileBase uploadedFile, string imageFileName)
{
    if (Path.GetExtension(imageFileName).Equals(".svg", StringComparison.OrdinalIgnoreCase))
    {
        if (uploadedFile.ContentLength > 0)
        {
            byte[] data;
            using (Stream inputStream = uploadedFile.InputStream)
            {
                Stream inputStream = uploadedFile.InputStream;
                var memoryStream = inputStream as MemoryStream;
                if (memoryStream == null)
                {
                    memoryStream = new MemoryStream();
                    inputStream.CopyTo(memoryStream);
                }

                data = memoryStream.ToArray();
            }

            var parsedData = Encoding.UTF8.GetString(data, 0, data.Length).TrimEnd('\0');
            var result = parsedData.ContainsAny(Constants.InsecureStrings, StringComparison.CurrentCultureIgnoreCase);
            return result;
        }
    }

    return false;
}

Can anyone help me to know how to fix this issue?

santosh kumar patro
  • 7,231
  • 22
  • 71
  • 143
  • How about if you put `var parsedData = Encoding.UTF8.GetString(data, 0, data.Length).TrimEnd('\0'); var result = parsedData.ContainsAny(Constants.InsecureStrings, StringComparison.CurrentCultureIgnoreCase);` to inside of `using` block? And `data`too ofc – Selim Yildiz Jan 27 '20 at 18:58
  • Thanks @SelimYıldız for your response. Can you please help me to know what does And data too ofc mean here. – santosh kumar patro Jan 27 '20 at 19:13
  • I mean put `byte[] data;` and `var parsedData = Encoding.UTF8.GetString(data, 0, data.Length).TrimEnd('\0'); var result = parsedData.ContainsAny(Constants.InsecureStrings, StringComparison.CurrentCultureIgnoreCase); return result;` to inside of `using` block. – Selim Yildiz Jan 27 '20 at 19:15
  • I've updated my answer to show how to work around the issue, roughly, with the above code. The core problem is detailed in the answer of this question https://stackoverflow.com/questions/16030034/asp-net-mvc-read-file-from-httppostedfilebase-without-save – RamblinRose Jan 28 '20 at 00:11

1 Answers1

1

First to address your issue, which I now understand is that after the call to ValidateUploadedImageContent the image stream is invalid.

That is because the stream gained from the HttpPostedFileBase is "read-only sequential (non-seekable)" as detailed in this SO answer. This explains why the stream's ContentLength is 0 - the stream has been consumed by the validation call.

If you have flexibility with the ImageDTO class, modifying the validation method such that it returns the image bytes would be a workaround.

For example,

// on success, buffer contains the image data. Otherwise it is null.
private bool ValidateUploadedImageContent(
    out byte[] buffer,
    HttpPostedFileBase uploadedFile,
    string imageFileName)
{
    buffer = null;
    if (Path.GetExtension(imageFileName).Equals(".svg", StringComparison.OrdinalIgnoreCase))
    {
        if (uploadedFile.ContentLength > 0)
        {
            var reader = new BinaryReader(inputStream);
            buffer = reader.ReadBytes((int)uploadedFile.ContentLength);
            var parsedData = Encoding.UTF8.GetString(buffer, 0, buffer.Length).TrimEnd('\0');
            return parsedData.ContainsAny(Constants.InsecureStrings, StringComparison.CurrentCultureIgnoreCase);
        }
    }
    return false;
}

I've used BinaryReader to simplify the code.

Then back to the calling method,

byte[] imageBuffer = null;

if (model != null && model.image.ContentType.Contains(Constants.Image) 
    && !ValidateUploadedImageContent(out imageBuffer, model.image, model.image.FileName)) {

    var dto = new ImageDTO();
    using(var imageStream = new MemoryStream(imageBuffer)) {
        // pass along imageStream to your ImageDTO and save.
    }        
}

Again, hopefully you have some flexibility with the ImageDTO class.

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
RamblinRose
  • 4,883
  • 2
  • 21
  • 33
  • Would be good answer if you actually explain why it (indeed) fixes the issue... (Obviously not because all the confusing BinaryReader code in the updated code block, `MemoryStream` [code](https://stackoverflow.com/questions/221925/creating-a-byte-array-from-a-stream) is just as good) – Alexei Levenkov Jan 27 '20 at 22:25
  • @AlexeiLevenkov indeed a very fair comment - this answer really deserves a down vote, at least until I provide a complete answer. – RamblinRose Jan 27 '20 at 23:07
  • Nice edit! I actually thought that your original `.Position = 0` was the answer, but indeed it would not help as theses particular streams are not seekable. – Alexei Levenkov Jan 28 '20 at 06:09
  • @AlexeiLevenkov for your exemplary comment the answer is most improved - thanks. – RamblinRose Jan 28 '20 at 14:50