Hide Forgot
Created attachment 551714 [details] ss1 Description of problem: env -> pool -> new image maliciously upload any kind of file.. a png, or other binary file. File does get uploaded... Wondering if this is a security risk?? See screenshots
Created attachment 551715 [details] ss2
I'm not entirely sure what the added risk is; from what I can tell (although I may be missing something), it's not possible to tell if a file is binary without reading it, so the upload would have to go through either way. . . ? Unless there's an alternative process that I'm missing. Mainn
adding to ce-sprint
removing ce-sprint-next tracker
Kurt, can we get a security opinion on how severe a problem this is? We're talking about a user's ability to upload a file claiming it is an XML document when it could in fact be anything (and we don't check to see what it is). We would never sent the file to image factory as actual build instructions, because it wouldn't pass XML validation, but it would be in the system. Thanks, --Hugh
Some possible problems this creates: 1) additional attacks against the XML parser beyond simple XML attacks, e.g. binary data that triggers logging errors/etc. 2) if the files are stored in an accessible location they could be used in conjunction with other attacks, e.g. a local file inclusion flaw in the web interface or something, or local vulns (pass a malformed file to something) 3) Do the files eventually get cleaned out somehow? If not they could result in usage of disk space or inodes.
I agree that it can be a danger, but I don't see how it's possible to analyze the file without actually uploading the file. Just looking at the extension isn't reliable. Are we just looking to make sure that a file is deleted if it's not xml, or. . . ?
Ideally the file should be examined once uploaded and if not XML deleted and an error thrown back to the user (they may have selected the wrong file, a corrupted file, etc.). Obviously malformed XML can be uploaded (Turing and all that) but at least we avoid the obvious garbage.
We should be fine then - the code that reads the XML is this: file = params[:image_file] xml_source = file && file.read As far as I can tell, this doesn't actually save the file; worst case, if the file is over a certain size, it creates a temp file that is removed after the request ends. Does that satisfy this issue. . . ?
determined not to be a bug..