search

aguidrevitch / jquery-file-upload-middleware

jQuery-File-Upload Express.js middleware
294 stars 120 forks source link

Avoid path traversal in DELETE requests #56

Closed derjust closed 10 years ago

derjust commented 10 years ago

The DELETE request handling can be used to delete any file on the server as long as the node process has access to it as the fileName parameter is not properly checked from the client.

This fix ensures that the desired file in the upload folder.

aguidrevitch commented 10 years ago

I guess validation of versionfilepath is redundant, otherwise looks reasonable. Merged in

aguidrevitch commented 10 years ago

Also, it would be safer to ignore errors when removing image versions - they should not result in global failure

derjust commented 10 years ago

Hm. But without the if (versionfilepath.indexOf(options.uploadDir()) !== 0) { the path traversal might occur again. Based on HEAD it is considered for "normal" files. But if versions are used/activated and a filename like '../../../../etc/password' from the client, an unitended file is deleted. path.join just concats the strings and resolves all ../ parts - but on a string based level. No FS-checks itself are performed.

aguidrevitch commented 10 years ago

According to current logics, the script will return in the block if (filepath.indexOf(options.uploadDir()) !== 0) and never hit the code with versionfilepath. Also, options.imageVersions are under developer's control, so everything looks completely safe.