TeamMentor / TM_4_0_Design

Repo Holds TM 4.x issues
4 stars 3 forks source link

Add Unit Test to confirm that there are no raw-html outputs inside all Jade Pages #385

Open DinisCruz opened 9 years ago

DinisCruz commented 9 years ago

As far as I know there are two ways Jade can produced un-encoded content (i.e raw html)

Both should never be used, since a key feature of our use of Jade is to have a 'by default' encoding html environment.

@tekgirl77 can you have a go at this one (should be a bit easier than the SSL you are looking at)

Here are some tasks that will need to be done as part of this issue:


TM-4.0-Security

romichg commented 9 years ago

@tekgirl77 feel free to adjust the level of effort that you think it may take you.

tekgirl77 commented 9 years ago

I will take a look! :)

Busy day w/ support today (Colin's out sick) so I haven't had a chance to work on TM today.

Salle Ingle Support & Implementation Specialist, eLearning [image: https://www.securityinnovation.com/_img/art/logo.png]

On Tue, Feb 3, 2015 at 3:19 PM, RomichG notifications@github.com wrote:

@tekgirl77 https://github.com/tekgirl77 feel free to adjust the level of effort that you think it may take you.

— Reply to this email directly or view it on GitHub https://github.com/TeamMentor/TM_4_0_Design/issues/385#issuecomment-72736985 .

DinisCruz commented 9 years ago

Here is an example of why this test is so important https://github.com/TeamMentor/TM_4_0_Design/blob/Dev/source/jade/_mixins/user-mixins.jade#L64-L94

When coding in new frameworks it is so easy to create vulns like that

DinisCruz commented 9 years ago

Here is the commit that uses Jade's auto encoding capabilities: https://github.com/TeamMentor/TM_4_0_Design/commit/cb7690f51852a9282f273ef58df0445b171e8d64#diff-0

tekgirl77 commented 9 years ago

@DinisCruz here is the preliminary code for #385; is this on the right track?
https://github.com/tekgirl77/TM_4_0_Design/tree/Issue_385_No_jade_raw_html

It finds the unescape chars in all jade files, but as of now is just printing them out to the console.
https://travis-ci.org/tekgirl77/TM_4_0_Design/builds/49891022

I'd like to touch base w/ you before proceeding to make sure I'm on the right track and also get some direction on writing the respective assertions. Thanks!!

DinisCruz commented 9 years ago

Hi @tekgirl77 when asking questions like the above it helps if you: a) link directly to the affected source code, like this: or 2) include in the comment the affected source code, like I did below (see the source of this comment for how to include source code snippets in comments)

    it.only 'Issue_385_No_jade_raw_html', (done)->
      jadeService = new Jade_Service();
      jadeParentDir = jadeService.repo_Path.path_Combine('/source/jade/')

      recursiveWalk = (parentDirPath, filelist)->
        filelist = filelist || []
        files = fs.readdirSync(parentDirPath)
        console.log "files are " + files
        files.forEach (file)->
          if fs.statSync(parentDirPath + file).isDirectory()
            filelist = recursiveWalk(parentDirPath + file + "/", filelist)
          else
            filelist.push parentDirPath + file
          return
        filelist

      jadeFiles = recursiveWalk(jadeParentDir, filelist = [])
      for file in jadeFiles
        jadeService.findJadeUnescapeChars(file)
      done()

and

    findJadeUnescapeChars: (jadeFile)->
      readline = require('linebyline')
      fileContents = readline(jadeFile)
        .on 'line', (line)->
          if line.trim().charAt(0) == '|'
            console.log "Found the pipe at " + jadeFile + "\n" + line
          for word in line.trim().split(" ")
            if word.charAt(word.length-1) == '!'
              console.log "Found the ! at " + jadeFile + "\n" + line + "\n" + word
        .on 'error', (err)->
          throw err
DinisCruz commented 9 years ago

In terms of the direction of the solution, yes you are on the right track :)

And it is good that you got your head around node/js to write those helper functions, but the reason I created fluentnode was to make code that like much simpler :)

So, can you use the functions below in your code? (which will make it simpler and easier to read):

tekgirl77 commented 9 years ago

I knew there were helper functions available for the recursive walk but I wanted to write it myself for the practice w/ coffeescript :)

I'll take a look when I get a chance... starting my work with django and will be splitting my time.

Thanks for the quick feedback!! Hope you're having a great weekend! :)

Salle Ingle Support & Implementation Specialist, eLearning [image: https://www.securityinnovation.com/_img/art/logo.png]

On Sun, Feb 8, 2015 at 8:58 AM, Dinis Cruz notifications@github.com wrote:

In terms of the direction of the solution, yes you are on the right track :)

And it is good that you got your head around node/js to write those helper functions, but the reason I created fluentnode was to make code that like much simpler :)

So, can you use the functions below in your code? (which will make it simpler and easier to read):

— Reply to this email directly or view it on GitHub https://github.com/TeamMentor/TM_4_0_Design/issues/385#issuecomment-73414246 .

DinisCruz commented 9 years ago

I knew there were helper functions available for the recursive walk but I wanted to write it myself for the practice w/ coffeescript :)

Yap that is a great way to learn :)

Btw, if you think that your implementation works better than the correspondent fluentnode metho, please make sure to submit them as a PR

I also like that you are doing your tests using Unit Tests (and there is nothing wrong with creating a couple unit tests whose only purpose is to help to understand how a particular API works (I do that all the time))