Robert (Community Member) asked a question.

June 24, 2021 at 5:06 PM
CWE-80 - jQuery -- probable false positive?

Just wanted to post this one to see if anyone had any suggestions (other than schedule a consultation, or mark as "Mitigated: False Positive"):

 

Code:

 

  function calculateMainContainerSize() {

    var offset = 0;

    var elements = $('#page-search-container, #claim-info-header');

    elements.each(function (index, e) {

      var element = $(e); // line 50

      if (element.length && element.outerHeight() !== null) {

        offset += element.outerHeight();

      }

    });

    return 'calc(100vh - ' + offset + 'px)';

  }

 

Flaw: on line 50, "var element = $(e);"

CWE-80: This call to jQuery() contains a cross-site scripting (XSS) flaw. ....etc....

 

Screenshot showing it on the triage flaws page:

Screen Shot 2021-06-24 at 13.00.00 

  • 7 answers
  • 1.17K views

  • Boy, Security Consultant (Veracode)

    Hi @Robert (Community Member)​ ,

     

    Unfortunately, both look like false positives. Have you scanned this on the latest version of Veracode Static Analysis? We have recently made significant improvements to our JavaScript modelling.

     

    I would recommend you contact our technical support team. Here's how you can log a case:

    1. Navigate to the upper right corner of any page in the Community, click on your user avatar.

    2. Select Contact Support from the drop-down menu.

     

    Typically, we don't recommend making code changes to workaround false positive issues, rather we'd recommend documenting this in a mitigation. However, as you requested alternatives you could consider rewriting the functionality without jQuery to something like this:

     

    function calculateMainContainerSizeSansJQuery() {

    let offset = 0;

    [

    document.getElementById('page-search-container'),

    document.getElementById('claim-info-header')

    ].forEach((e) => {

    var element = $(e); // line 50

    if (element.length && element.outerHeight() !== null) {

    offset += element.outerHeight();

    }

    })

    return 'calc(100vh - ' + offset + 'px)';

    }

     

    This is an excellent resource to learn more about jQuery alternatives: http://youmightnotneedjquery.com .

     

    Thank you,

    Boy Baukema

    Expand Post
    Selected as Best

  • Robert (Community Member)

    And another similar one that I cannot for the life of me understand where it thinks the taint source is.Screen Shot 2021-06-24 at 13.13.59It's flagging the "append" as the the jQuery method that has an XSS flaw. In this case, the $() call is taking a completely static string, and the .text() call should be considered a cleansing function. Unfortunately, there never seems to be a "data path" listed for these problems, so we cannot easily tell where the scanner thinks the source of the tainted data is. Maybe it's from line 1651 (item.label)? That's the only thing I can think of.

     

    Insights welcome...

    Expand Post

  • Boy, Security Consultant (Veracode)

    Hi @Robert (Community Member)​ ,

     

    Unfortunately, both look like false positives. Have you scanned this on the latest version of Veracode Static Analysis? We have recently made significant improvements to our JavaScript modelling.

     

    I would recommend you contact our technical support team. Here's how you can log a case:

    1. Navigate to the upper right corner of any page in the Community, click on your user avatar.

    2. Select Contact Support from the drop-down menu.

     

    Typically, we don't recommend making code changes to workaround false positive issues, rather we'd recommend documenting this in a mitigation. However, as you requested alternatives you could consider rewriting the functionality without jQuery to something like this:

     

    function calculateMainContainerSizeSansJQuery() {

    let offset = 0;

    [

    document.getElementById('page-search-container'),

    document.getElementById('claim-info-header')

    ].forEach((e) => {

    var element = $(e); // line 50

    if (element.length && element.outerHeight() !== null) {

    offset += element.outerHeight();

    }

    })

    return 'calc(100vh - ' + offset + 'px)';

    }

     

    This is an excellent resource to learn more about jQuery alternatives: http://youmightnotneedjquery.com .

     

    Thank you,

    Boy Baukema

    Expand Post
    Selected as Best

    • Robert (Community Member)

      Thanks Boy -- I was suspecting so. This is with the latest engine -- I ran about 7 or 8 scans last night to try to narrow down the problem and what triggers it, and how to work around it. I think I have some ideas of how to avoid it, but it's not consistent.

       

      The "fun" part is when I make a few go away, it starts finding a few more...again all false positives. I have about 15-20 of them, and every time I make a few "go away", a few more start showing up.

       

      This area of the engine is definitely struggling with our code base.

       

      We are slowly moving away from jQuery, but moving large existing projects takes a long time.

       

      I can raise support ticket -- they usually have no idea, and ask me to schedule a consultation. Which I then do, and then the consultant will tell me the usual thing, which it's another false positive, and they will raise to Ops to have a look.

       

      Really, I'd like to stop going through that process each time and just escalate direct to Ops to confirm the issue -- I will try doing so via a support ticket (as ultimately that what the Ops ticket will be anyway).

       

      Here's another example, and some variations I was testing:

       

          // START: Veracode static scanner testing

          //  Flags .after() (as flaw id 17538, CWE-80, XSS injection)

          $("#show_phase_changes").after($("<label></label>", {

            for: "show_phase_changes"

          }).text(EX.Messages.prop("Show_phase_changes")));

          // Test #1

          // After commenting out Test #3, flaw moved to Test #1

          //  flagged the last line of this test, with .after() (flaw id 17538 - reused the original number)

          var label = $("<label></label>", {

            for: "show_phase_changes"

          }).text(EX.Messages.prop("Show_phase_changes"));

          $("#show_phase_changes").after(label);

          // Test #2

          var label2 = $("<label></label>", {

            for: "show_phase_changes"

           });

          label2.text(EX.Messages.prop("Show_phase_changes"));

          $("#show_phase_changes").after(label2);

          // Test #3

          // flagged last line of this test, with .after() (flaw id 17553)

          var label3 = $("<label></label>", {

            for: "show_phase_changes"

          });

          label3.text(EX.Messages.prop("Show_phase_changes"));

          var showPhaseChanges = $("#show_phase_changes");

          showPhaseChanges.after(label3);

          // END: Veracode static scanner testing

       

       

      I eventually settled on this to "fix" this specific instance:

       

          showPhaseChangesInput.appendTo("#notes_table_wrapper .top");

          var showPhaseChangesLabel = $("<label></label>", {

            for: "show_phase_changes",

            text: EX.Messages.prop("Show_phase_changes")

          });

          showPhaseChangesInput.after(showPhaseChangesLabel);

       

       

      Which is at least easy to read... 🙂

       

      Expand Post

      • Robert (Community Member)

        Ticket raised with support.

      • Boy, Security Consultant (Veracode)

        I'm sorry to hear you're frustrated with the current process. Unfortunately, Veracode Technical Support, while very versatile, do not have a security and development background and any FPs must first be vetted by a the ASC team (who does have this background) before being reviewed by Veraocde Engineering.

         

        If you're having persistent issues you may want to ask an ASC if you can simply email newer examples and continue to work with the ASC. In general we try to not be very 'sticky' so as to give you the greatest availability (after all a given ASC can be out sick, on PTO, or on other tasks) but for some persistent issues this may work best. Please feel free to discuss this with an ASC you've spoken to (feel free to reach out to me if you do not already have contact details for an ASC).

         

        Thank you,

        Boy Baukema

        Expand Post

      • Robert (Community Member)

        Understood. I did discuss some of these challenges around the ASC process and when it "ends" with Bill T and Jim J a couple of weeks back, so they are aware of my concerns. I try not to bother the ASCs directly except as it relates to any specific consultation session we had with them (and any outstanding actions from that session) because I don't know exactly how their time is allocated.

         

        I do have contact information for all the ASCs we've worked with in the past, so I could always reach out to them, but there has been some time I think between initially presenting this issue, and the more recent recurrences of it. I'll see how support does first -- I've got a response. I also made Jim J and my account advisor aware of the ticket so they might be able to help direct it accordingly inside the organization.

        Expand Post

    • Robert (Community Member)

      Just one minor correction to the suggested changes @Boy, Security Consultant (Veracode)​ suggested, is that "line 50" also needs changing to `var element = e;` (or that could be eliminated entirely and rename variables). That would complete the removal of jQuery from that section of code.

Topics (8)

No articles found
Loading

Ask the Community

Get answers, share a use case, discuss your favorite features, or get input from the community.