Skip to main content
added 1289 characters in body
Source Link
greggles
  • 5.3k
  • 21
  • 37

This should really be addressed to the fmath project maintainers. I've mailed them to point out some security concerns in the code. If they don't respond I'll edit this post to answer the question here, but hopefully they will respond with a new version ;)

Original comment - mailed to fmath maintainer:

Some key things this is missing that you would want to get right:

  • CSRF protection - If you don't use the form API (which includes this already) then you can manually use drupal_get_token and drupal_valid_token are helpers.
  • Malicious file types - file_munge_filename is a helper here
  • Too many/too big of files
  • Temporarily uploading the file to a temporary directory outside of the webroot and giving it a random name so that it can be validated in that temporary location before moving it to the final location
  • Ideally you would validate the file contents (e.g. virus scan, confirm the file is a valid file of whatever the extension/mime type says it is).
  • Control where the file can be placed.

This script looks like it would only allow jpg or png files to be uploaded, but it misses out on a few other security features

There are probably a few other things to worry about, but I think that list is a good start.

I also wonder about this piece and whether you could sneak base64 encoded html into a "jpg" file and then some veresions of internet explorer and safari will do mime type detection and execute the html (i.e. cross site scripting attack):

   echo base64_decode($_POST["image"]);

This should really be addressed to the fmath project maintainers. I've mailed them to point out some security concerns in the code. If they don't respond I'll edit this post to answer the question here, but hopefully they will respond with a new version ;)

This should really be addressed to the fmath project maintainers. I've mailed them to point out some security concerns in the code. If they don't respond I'll edit this post to answer the question here, but hopefully they will respond with a new version ;)

Original comment - mailed to fmath maintainer:

Some key things this is missing that you would want to get right:

  • CSRF protection - If you don't use the form API (which includes this already) then you can manually use drupal_get_token and drupal_valid_token are helpers.
  • Malicious file types - file_munge_filename is a helper here
  • Too many/too big of files
  • Temporarily uploading the file to a temporary directory outside of the webroot and giving it a random name so that it can be validated in that temporary location before moving it to the final location
  • Ideally you would validate the file contents (e.g. virus scan, confirm the file is a valid file of whatever the extension/mime type says it is).
  • Control where the file can be placed.

This script looks like it would only allow jpg or png files to be uploaded, but it misses out on a few other security features

There are probably a few other things to worry about, but I think that list is a good start.

I also wonder about this piece and whether you could sneak base64 encoded html into a "jpg" file and then some veresions of internet explorer and safari will do mime type detection and execute the html (i.e. cross site scripting attack):

   echo base64_decode($_POST["image"]);
Source Link
greggles
  • 5.3k
  • 21
  • 37

This should really be addressed to the fmath project maintainers. I've mailed them to point out some security concerns in the code. If they don't respond I'll edit this post to answer the question here, but hopefully they will respond with a new version ;)