cracker0dks / whiteboard

Lightweight collaborative Whiteboard / Sketchboard
MIT License
719 stars 198 forks source link

Security improvements: protect from directory traversal and iFrame content injection #107

Closed lightswitch05 closed 3 years ago

lightswitch05 commented 3 years ago

I've included two security related bug-fixes in this PR:

  1. Improve protections from directory traversal when using file database option

Using path.dirname, we can get the path from the generated filePath. If path.dirname does not match FILE_DATABASE_FOLDER, then we know that the given wid contains path information and should not be trusted.

Example:

var wid = '../../example';
var fileName = wid + ".json";
var filePath = FILE_DATABASE_FOLDER + "/" + fileName;
path.dirname(filePath); // "savedBoards/../.." != "savedBoards"

Using path.basename, we can get the filename from the generated filePath. If the path.basename does not match fileName, then we know that the given wid contains path information and should not be trusted.

Example:

var wid = '../../example';
var fileName = wid + ".json";
var filePath = FILE_DATABASE_FOLDER + "/" + fileName;
path.basename(fileName); // "example.json" != "../../example.json"
  1. Prevent iFrames/popups from injecting content into the whiteboard through fake 'paste' events

A malicious iframe or popup link could generate a false "paste" event using the Window.postMessage API. A false event would have event.origin set to be the iFrame's URL. Real paste events have an undefined event.origin. This PR ensure event.origin is unset before injecting the content into the Whiteboard. Read more here: https://sonarcloud.io/organizations/lightswitch05/rules?open=javascript%3AS2819&rule_key=javascript%3AS2819

lightswitch05 commented 3 years ago

More SonarCloud scan results can be found here: https://sonarcloud.io/dashboard?id=lightswitch05_whiteboard

cracker0dks commented 3 years ago

Thanks for the patch! appreciated 👍

lightswitch05 commented 3 years ago

Thanks for merging! I'm sorry about the styling- I thought I had that figured out, I guess not.