0

I have the following delete function in expressjs but when I try use the path of the element I want to remove it appears as undefined yet I've revised the tutorial and still dont see where the problem is:

router.delete('/messagedelete/:empId', function (req, res) {

  Message.remove({empId: req.params.empId}, function(err, message) {
      console.log(message.path);
      console.log("got inside");
    if(err) { 
       return res.send({status: "200", response: "fail"});
    }
      console.log(message.path);
    fs.unlink(message.path, function() {
      res.send ({
        status: "200",
        responseType: "string",
        response: "success"
      });     
    });
 }); 
});

I import fs like this at the top of the file:

const fs = require('fs');
user7629970
  • 91
  • 1
  • 1
  • 8

1 Answers1

1

There are two main problems:

  1. You are not handling the errors
  2. You don't check what file would get deleted

Change this:

fs.unlink(message.path, function() {
  res.send ({
    status: "200",
    responseType: "string",
    response: "success"
  });     
});

to:

fs.unlink(message.path, function (err) {
  if (err) {
    // handle the error - like res.send with status 500 etc.
  }
  res.send ({
    status: "200",
    responseType: "string",
    response: "success"
  });     
});

You didn't include any info what is the value of message.path but you should never delete anything if you're not 100% sure you know what is the path and here your program has really no idea what it's trying to delete - it could be its own source code or some other important file on the system for all we know.

You should use path.join() to join some prefix of where you want to delete the files and the message.path that you got, with something like:

let filePath = path.join(dir, message.path);

where dir is a place where you keep the files you want deleted - if you don't do it then you may be deleting files on the entire filesystem. But that's not enough, you actually have to check if the result in filePath is not outside of dir which it well may be if the message.path contained .. for example. So you also need for example:

if (filePath.indexOf(dir + path.sep) !== 0) {
    return res.status(403).end('Forbidden');
}

See this answer for more examples and more details on validating paths that you compose with path.join and why it's important:

Community
  • 1
  • 1
rsp
  • 107,747
  • 29
  • 201
  • 177
  • But how are you meant to get the file path of a mongodb doc? im not deleting off the computer im interacting with a mongodb I set up – user7629970 Apr 04 '17 at 20:49
  • I removed fs unlink as I dont think its really needed here as the tutorial was a deleting with a express tutorial and didnt really involve mongoose, the methods working fine now ill just have to clean it up a bit, think fs unink was just added more problems then needed – user7629970 Apr 04 '17 at 21:05