Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spurious error appeared suddenly in 1.20.0 #7694

Closed
1 of 3 tasks
msorens opened this issue May 1, 2023 · 6 comments · Fixed by #7789
Closed
1 of 3 tasks

Spurious error appeared suddenly in 1.20.0 #7694

msorens opened this issue May 1, 2023 · 6 comments · Fixed by #7789
Assignees
Labels

Comments

@msorens
Copy link

msorens commented May 1, 2023

Describe the bug

Having just upgrade to v1.20.0 and having changed no files in the repo, these errors just started appearing.
These errors were NOT present with v1.19.0.
Note that line 1 of each of the 2 noted files was a standard JavaScript import statement (import PropTypes from 'prop-types')

% semgrep -f p/cwe-top-25                     
      
. . . standard info left out here...
                                                                                                                        
  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╸ 100% 0:00:01                                                                                                                        
[ERROR] Fatal error at line src/components/system-view.jsx:1:
 (Failure "Call AST_utils.with_xxx_equal to avoid this error.")
====[ BEGIN error trace ]====
(Failure "Call AST_utils.with_xxx_equal to avoid this error.")
=====[ END error trace ]=====

[ERROR] Fatal error at line src/components/policy-authoring/aux-container.jsx:1:
 (Failure "Call AST_utils.with_xxx_equal to avoid this error.")
====[ BEGIN error trace ]====
(Failure "Call AST_utils.with_xxx_equal to avoid this error.")
=====[ END error trace ]=====

The same issue appears when running it just on either single file in question.
Note the mention of the internal semgrep errors was present with previous semgrep versions; it is just the fatal error that is new with 1.20.0.

% semgrep -f p/cwe-top-25 src/components/system-view.jsx
               
               
┌─────────────┐
│ Scan Status │
└─────────────┘
  Scanning 1 file tracked by git with 207 Code rules:
                                                                                                                        
  Language      Rules   Files          Origin      Rules                                                                
 ─────────────────────────────        ───────────────────                                                               
  <multilang>       3       2          Community     207                                                                
  js               41       1                                                                                           
                                                                                                                        
  ━━━━━━━━━━━━━━━━━━━━━━━━━━╸━━━━━━━━━━━━━  67% 0:00:00                                                                                                                        
[ERROR] Fatal error at line src/components/system-view.jsx:1:
 (Failure "Call AST_utils.with_xxx_equal to avoid this error.")
====[ BEGIN error trace ]====
(Failure "Call AST_utils.with_xxx_equal to avoid this error.")
=====[ END error trace ]=====
              
┌──────────────┐
│ Scan Summary │
└──────────────┘
Some files were skipped or only partially analyzed.
  Partially scanned: 1 files only partially analyzed due to parsing or internal Semgrep errors

To Reproduce
Apparently just need to supply a file that causes Semgrep errors.

Expected behavior
No error.

What is the priority of the bug to you?

  • P0: blocking your adoption of Semgrep or workflow
  • P1: important to fix or quite annoying
  • P2: regular bug that should get fixed

Environment
If not using semgrep.dev: are you running off docker, an official binary, a local build?
Both local binary and docker versions do the same thing.

@ocean
Copy link

ocean commented May 2, 2023

I've just had this error as well, in my CI pipeline, on "line 1" of a plain JavaScript file, which only contains a use strict statement.

[ERROR] Fatal error at line /src/web/modules/custom/jjj_utils/js/jjj_utils.js:1:
 (Failure "Call AST_utils.with_xxx_equal to avoid this error.")
====[ BEGIN error trace ]====
(Failure "Call AST_utils.with_xxx_equal to avoid this error.")
=====[ END error trace ]=====

The start of the file:

'use strict';

/**
 * @file
 * Contains global functions we can call from other modules.
 */

(function($) {
  ...

@ievans ievans added bug Something isn't working javascript labels May 2, 2023
@IagoAbal
Copy link
Collaborator

IagoAbal commented May 3, 2023

@msorens @ocean could any of you provide a file to reproduce this issue?

@ocean
Copy link

ocean commented May 4, 2023

@IagoAbal The file it choked on for me is an IIFE that uses JQuery. It's old and needs updating, but does all run fine.

'use strict';

/**
 * @file
 * Contains global functions we can call from other modules.
 */

(function($) {

  if (typeof window.jjj === 'undefined') {
    window.jjj = {};
  }

  var jjj = window.jjj;

  jjj.DESKTOP_SIZES = ['xl', 'lg'];
  jjj.TABLET_SIZES = ['md', 'sm'];
  jjj.MOBILE_SIZE = 'xs';

  // -------------------------------------------------------------------------------------------------------------------
  // Global jjj functions.

  /**
   * Check if the key pressed matches the key code string. Only works with a limited set of key values (see below).
   *
   * For all key values:
   * @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key/Key_Values
   *
   * @param {Object} event
   *   The jQuery event object.
   * @param {string} key
   *   A value representing the key (e.g. 'Enter', 'ArrowLeft', etc.)
   *
   * @return {boolean}
   *   If the key pressed was Enter.
   */
  jjj.keyIs = function(event, key) {
    var keyCodes = {
      Enter: 13,
      ' ': 32,
      End: 35,
      Home: 36,
      ArrowLeft: 37,
      ArrowUp: 38,
      ArrowRight: 39,
      ArrowDown: 40
    };

    // Check 'key' property. This is modern, and best practice for new websites, but not available in all browsers yet.
    // @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key
    // @see https://caniuse.com/#feat=keyboardevent-key
    if (typeof event.key !== 'undefined' && event.key === key) {
      return true;
    }

    // Check 'which' property. Available in many browsers, but deprecated.
    // @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/which
    // @see https://caniuse.com/#feat=keyboardevent-which
    if (typeof event.which !== 'undefined' && event.which === keyCodes[key]) {
      return true;
    }

    // Check 'charCode' property. Available in many browsers, but deprecated.
    // @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/charCode
    // @see https://caniuse.com/#feat=keyboardevent-charcode
    if (typeof event.charCode !== 'undefined' && event.charCode === keyCodes[key]) {
      return true;
    }

    // Check 'keyCode' property. Available in all popular browsers, but deprecated.
    // @see https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode
    if (typeof event.keyCode !== 'undefined' && event.keyCode === keyCodes[key]) {
      return true;
    }

    return false;
  };

  /**
   * Applies the same behaviour to an element's click event if Enter pressed. Useful for divs with button roles.
   *
   * @param {Object} event
   *   The jQuery event object.
   */
  jjj.clickIfEnterPressed = function(event) {
    if (jjj.keyIs(event, 'Enter')) {
      $(this).click();
      event.preventDefault();
    }
  }

  /**
   * Get the user's browser width as a 2-letter code, matching those used by Bootstrap.
   *
   * @return {string}
   *   The browser size code as used by Bootstrap.
   */
  jjj.getBrowserSizeCode = function() {
    if (window.matchMedia('(min-width: 1200px)').matches) {
      return 'xl';
    }
    if (window.matchMedia('(min-width: 960px)').matches) {
      return 'lg';
    }
    if (window.matchMedia('(min-width: 800px)').matches) {
      return 'md';
    }
    if (window.matchMedia('(min-width: 640px)').matches) {
      return 'sm';
    }
    return 'xs';
  };

  /**
   * Detects if we're on mobile.
   *
   * @return {boolean}
   *   TRUE if we're on mobile.
   */
  jjj.isMobile = function() {
    return jjj.getBrowserSizeCode() === 'xs';
  };

  /**
   * Make a form field required.
   *
   * @param {string} formGroups
   *   jQuery selector for affected form fields, identified by their enclosing form groups.
   */
  jjj.makeRequired = function(formGroups) {
    var $formGroups = $(formGroups);
    // Check each form group.
    $formGroups.each(function(i, el) {
      var $formGroup = $(el);

      // Add the "required" classes and attributes.
      if ($formGroup.hasClass('form-composite')) {
        // It's a radio button or checkbox.
        if ($formGroup.hasClass('form-composite-radios')) {
          // It's radios
          $formGroup.find('input[type=radio]').attr({ required: 'required', 'aria-required': 'true' });
        } else {
          $formGroup.attr({ required: 'required', 'aria-required': 'true' });
        }
        $formGroup.addClass('required');
        $formGroup.find('legend > span').addClass('js-form-required form-required');
      } else {
        // It's a text field, textarea, or select box.
        $formGroup.find('label').addClass('js-form-required form-required');
        $formGroup
          .find('input, select, textarea')
          .addClass('required')
          .attr({ required: 'required', 'aria-required': 'true' });
      }

      // Hide the "(Optional)" tag, if present.
      $formGroup.find('.optional').addClass('d-none');
    });
  };

  /**
   * Make a form field optional.
   *
   * @param {string} formGroups
   *   jQuery selector for affected form fields, identified by their enclosing form groups.
   */
  jjj.makeOptional = function(formGroups) {
    var $formGroups = $(formGroups);
    // Check each form group.
    $formGroups.each(function(i, el) {
      var $formGroup = $(el);
      // Check if it's a radio button or checkbox group.

      // Remove the "required" classes and attributes.
      if ($formGroup.hasClass('form-composite')) {
        // It's a radio button or checkbox.
        if ($formGroup.hasClass('form-composite-radios')) {
          // Its radios.
          $formGroup.find('input[type=radio]').removeAttr('required').removeAttr('aria-required');
        } else {
          $formGroup.removeAttr('required').removeAttr('aria-required');
        }
        $formGroup.removeClass('required');
        $formGroup.find('legend span').removeClass('js-form-required form-required');
      } else {
        // It's a text field, textarea, or select box.
        $formGroup.find('label').removeClass('js-form-required form-required');
        var $inputs = $formGroup.find('input, select, textarea');
        $inputs.removeClass('required').removeAttr('required').removeAttr('aria-required');
      }

      // Show the "(Optional)" tag, if present.
      $formGroup.find('.optional').removeClass('d-none');
    });
  };

  // ------------------------------------------------------------------------------------------------------------------
  // Functions relating to form errors.

  /**
   * If a field value has an error, and is changed, check to see if the error still needs to be there.
   *
   * @param {Object} event
   *   The event object.
   */
  jjj.revalidateField = function(event) {
    var $el = $(event.target);
    var $formGroup = $el.closest('.form-group');

    // If there's no error, do nothing.
    var hasError = $formGroup.hasClass('has-error');
    if (!hasError) {
      return;
    }

    var value = $el.val();
    var isValid = true;

    // Check required.
    if ($el.prop('required') && !value) {
      isValid = false;
    }

    // Check pattern.
    var pattern = $el.attr('pattern');
    if (value && pattern && !RegExp('^' + pattern + '$').test(value)) {
      isValid = false;
    }

    // If the new value is valid, remove the error.
    if (isValid) {
      // Remove the error class and text from form group.
      $formGroup.removeClass('has-error');
      $formGroup.find('.invalid-feedback').remove();
      $formGroup.find('.sr-error').remove();
      // Remove the error and invalid classes, and the aria-invalid attribute.
      $el.removeClass('error is-invalid').removeAttr('aria-invalid');
    }
  };

  /**
   * If a radio button group has a 'required' error, and one is selected, remove the error.
   *
   * @param {Object} event
   *   The event object.
   */
  jjj.revalidateRadio = function(event) {
    var $el = $(event.target);
    var $container = $el.closest('.form-group');

    // If there's no error, do nothing.
    var hasError = $container.hasClass('error');
    if (!hasError) {
      return;
    }

    var $radios = $container.find('input');

    // If one is checked, then remove the error.
    if ($radios.filter(':checked').length) {
      // Remove error class and text from form group.
      $container.removeClass('error').removeAttr('aria-invalid');
      $container.find('.invalid-feedback').remove();
      $container.find('.sr-error').remove();
      // Remove error stuff from radios.
      $radios.removeClass('error').removeAttr('aria-invalid');
    }
  };

  /**
   * If a checkbox or set of checkboxes has a 'required' error, and one is checked, remove the error.
   *
   * @param {Object} event
   *   The event object.
   */
  jjj.revalidateCheckbox = function(event) {
    var $el = $(event.target);

    // Find the enclosing container and if it's a single checkbox or a group.
    var isGroup = true;
    var $container = $el.closest('.form-group');
    if (!$container.length) {
      $container = $el.closest('.checkbox');
      isGroup = false;
    }
    var hasError;

    if (isGroup) {
      // If there's no error, do nothing.
      hasError = $container.hasClass('error');
      if (!hasError) {
        return;
      }

      var $checkboxes = $container.find('input');

      // If the field is required and one is checked, then remove the error.
      if ($container.hasClass('required') && $checkboxes.filter(':checked').length) {
        // Remove error class and text from form group.
        $container.removeClass('error').removeAttr('aria-invalid');
        $container.find('.invalid-feedback').remove();
        $container.find('.sr-error').remove();
        // Remove error stuff from checkboxes.
        $checkboxes.removeClass('error').removeAttr('aria-invalid');
      }
    } else {
      // If there's no error, do nothing.
      hasError = $container.hasClass('has-error');
      if (!hasError) {
        return;
      }

      // Check for required.
      if ($el.hasClass('required') && $el.prop('checked')) {
        // Remove error class and text from the container div.
        $container.removeClass('has-error');
        $container.find('.invalid-feedback').remove();
        $container.find('.sr-error').remove();
        // Remove error stuff from checkbox.
        $el.removeClass('error').removeAttr('aria-invalid');
      }
    }
  };

  /**
   * Set up useful labels to serve screen readers, and removes errors on field blur if valid
   */
  jjj.autoRevalidate = function(context) {
    // Allow setting context for which this function will run
    if (typeof(context)==='undefined') context = document;
    // Update the label so that the validation errors get read out by SRs.
    $('.invalid-feedback', context).each(function(i, el) {
       // Find the target element we want to put the label on, which is the closest input field.
      var $error = $(el);
      var $formItem = $error.closest('.js-form-item');
      var $target = $formItem.find('input, select, textarea,.js-form-type-radio').eq(0);
      var contactTnC = false;
      var $form = $target.closest('form');

      if ($form.hasClass('jjj-contactus-form') && $target.attr('id') === 'edit-terms') {
        // Special Error handling for Contact us T&Cs
        $target.attr('aria-describedby', $(this).attr('id'));
      }
      else {
        // Set a useful label for SRs, which includes the validation error.
        $error.attr('aria-hidden', 'true');
        var error = 'Error: ' + $error.text().trim();
        var id = $target.attr('id');
        var idRadio;
        if ($target.hasClass('js-form-type-radio')) {
          idRadio = $target.parent().parent().parent().attr('id');
        }

        // Get the <label> element for the target control.
        var $label = $('label[for="' + id + '"]');
        var label;
        // If there is no <label>, insert one.
        if (!$label.length && typeof id !== 'undefined') {
          $label = $target.before('<label for="' + id + '"></label>');
          label = '';
        } else {
          label = $label.text().trim();
        }

        var srText;
        if (label) {
          srText = '';
        } else {
          // Check if there's an alternative label.
          var $label2;
          var label2 = '';
          // 1. For radio buttons, the label for the group is the <legend> tag.
          if (!label2) {
            $label2 = $formItem.find('legend');
            if ($label2.length) {
              label2 = $label2.eq(0).text().trim();
            }
          }
          // 2. Check the control's title attribute.
          if (!label2) {
            label2 = $target.attr('title');
          }
          // 3. Check the first label in the container.
          if (!label2) {
            $label2 = $formItem.find('label');
            if ($label2.length) {
              label2 = $label2.eq(0).text().trim();
            }
          }
          if (label2) {
            srText = label2;
          }
        }

        if (typeof idRadio !== 'undefined') {
          // Apend the error (and possibly also the description) to the SR text.
          srText = error.replace(':', ',');
          // Append description to the error if there is one, but only if it isn't set by aria-labelledby.
          var description = $formItem.find('.description').text();
          if (description && description.id !== $target.attr('aria-labelledby')) {
            srText += ', ' + description;
          }
          var $legend = $('#' + idRadio + ' legend');
          // Remove previous sr text.
          $('.sr-error', $legend).remove();
          // Insert the SR text element.
          $legend.append(' <span role="text" class="sr-only sr-error">' + srText + '</span> ');
        } else {
          // Apend the error (and possibly also the description) to the SR text.
          srText += ', ' + error.replace(':', ',');
          // Append description to the error if there is one, but only if it isn't set by aria-labelledby.
          var description = $formItem.find('.description').text();
          if (description && description.id !== $target.attr('aria-labelledby')) {
            srText += ', ' + description;
          }
          // Remove previous sr text.
          $('.sr-error', $label).remove();
          // Insert the SR text element.
          $label.append('<span role="text" class="sr-only sr-error">' + srText + '</span>');
        }
      }
    });

    // Revalidate controls on change.
    $('input, select, textarea', context).not('[type="radio"]').not('[type="checked"]').change(jjj.revalidateField);
    $('input[type="radio"]', context).change(jjj.revalidateRadio);
    $('input[type="checkbox"]', context).change(jjj.revalidateCheckbox);
  };

  /**
   * Check browser is IE.
   *
   * @return {boolean}
   *   TRUE if we're on IE.
   */
  jjj.isIE = function() {
    if (navigator.userAgent.indexOf('MSIE') !== -1 || navigator.appVersion.indexOf('Trident/') > 0) {
      return true;
    }
    return false;
  }

  jjj.isEdge = function() {
    return (navigator.userAgent.indexOf("Edge") > -1);
  }

  /**
   * Check device is IOS.
   * @returns {boolean}
   */
  jjj.isIOS = function() {
    return (/^(iPhone|iPad|iPod)/.test(navigator.platform));
  }

  /**
   * Close all the open modals.
   */
  jjj.closeAllModals = function() {
    try {
      $('.modal.show').modal('hide');
    } catch(error)
    {

    }
  }

  /**
   * Make an announcement via Drupal.announce.
   *
   * @param {string} text
   *   The text to announce.
   * @param {boolean} force
   *   If true, forces reading even if the text is the same.
   * @param {string} priority
   *   The value of the aria-live attribute.
   */
  jjj.announce = function(text, force, priority) {
    // Make sure the library is loaded.
    if (typeof Drupal.announce !== 'undefined') {
      // Default priority is 'polite'.
      if (priority !== 'assertive' && priority !== 'off') {
        priority = 'polite';
      }
      // Clear the element if forcing.
      if (force) {
        $('#drupal-live-announce').text('');
      }
      Drupal.announce(text, priority);
    }
  }

  // -------------------------------------------------------------------------------------------------------------------
  // jQuery extensions.

  $.fn.autoRevalidate = function() {
    if (this.length) {
      jjj.autoRevalidate(this);
    }
    return this;
  }

  /**
   * Restricts input for each element in the set of matched elements to the given inputFilter.
   *
   * @param {function} inputFilter
   *   Function to test if the input value is valid or not.
   *
   * @return {function}
   *   Delegate.
   */
  $.fn.inputFilter = function(inputFilter) {
    return this.on('input keydown keyup mousedown mouseup select contextmenu drop', function() {
      if (inputFilter(this)) {
        this.oldValue = this.value;
        this.oldSelectionStart = this.selectionStart;
        this.oldSelectionEnd = this.selectionEnd;
      } else if (this.hasOwnProperty('oldValue')) {
        this.value = this.oldValue;
        this.setSelectionRange(this.oldSelectionStart, this.oldSelectionEnd);
      }
    });
  };

  /**
   * Restricts input for each element in the set of matched elements to digits only.
   *
   * @return {function}
   *   Delegate.
   */
  $.fn.digitsOnly = function() {
    return this.inputFilter(function(element) {
      return /^\d*$/.test(element.value);
    });
  };

  /**
   * Enforces the 'maxlength' attribute for each element in the set of matched elements.
   *
   * This is needed because maxlength doesn't work on Chrome on Samsung Galaxy S7 (jjj-1764).
   *
   * @return {function}
   *   Delegate.
   */
  $.fn.maxlength = function() {
    return this.inputFilter(function(element) {
      var maxlength = parseInt($(element).attr('maxlength'), 10);
      // If maxlength not set, any value is fine.
      if (maxlength === 0 || isNaN(maxlength)) {
        return true;
      }
      // Check the value does not exceed the specified length.
      return element.value.length <= maxlength;
    });
  };

  /**
   * jQuery function to add keyboard navigation to tablists for accessibility.
   * https://www.w3.org/TR/wai-aria-practices-1.1/#kbd_roving_tabindex
   *
   */
  $.fn.rovingtabs = function() {
    return this.each(function() {
      var tabs = $("[role='tab']", this);
      var tabcount = tabs.length;
      tabs
        .on('focus', function() {
          $("[role='tab']:not(this)")
            .attr('aria-selected', 'false')
            .attr('tabindex', '-1')
            .removeClass('active');
          $(this).attr('aria-selected', 'true')
            .attr('tabindex', '0')
            .addClass('active');

          var tabpanid = $(this).attr('aria-controls');
          var tabpan = $('#' + tabpanid);

          $("div[role='tabpanel']:not(tabpan)")
            .attr('aria-hidden', 'true')
            .removeClass('active');
          tabpan.attr('aria-hidden', 'false')
            .addClass('active');
        })
        .on('keydown', function(e) {
          var index = tabs.index(this);
          if (jjj.keyIs(e, 'ArrowLeft') || jjj.keyIs(e, 'ArrowUp')) {
            // Left and Up.
            if (index > 0) {
              tabs[index - 1].focus();
            } else {
              tabs[tabcount - 1].focus();
            }
            e.preventDefault();
          } else if (jjj.keyIs(e, 'ArrowRight') || jjj.keyIs(e, 'ArrowDown')) {
            // Down or Right.
            if (index < tabcount - 1) {
              tabs[index + 1].focus();
            } else {
              tabs[0].focus();
            }
            e.preventDefault();
          }
        });
      // Set the initial selected one.
      $("[role='tab'].active", this).attr('aria-selected', 'true').attr('tabindex', '0');
    });
  };

  /**
   * Function to fix JAWS not announcing tab count on tabs in IE.
   * This should be called on an element which has role="tablist"
   *
   * @return {Function}
   *   jQuery function to add aria-labels for tablist count.
   */
  $.fn.tabcountFix = function() {
    if (jjj.isIE()) {
      // Get the label id.
      var labelledby = $(this).attr('aria-labelledby');
      // Get the label text.
      var label = $(this).parent().find('#' + labelledby).text();
      // Loop over each tab.
      return this.each(function() {
        var tabs = $("[role='tab']", this);
        var tabcount = tabs.length;
        tabs.each(function(index, element) {
          // Set element to a jquery object.
          element = $(element);
          // Set the index to be spoken.
          var i = index + 1;
          // Get the text of this tab.
          var title = element.text();
          element.attr('aria-label', label + ' ' + title + ' ' + i + ' of ' + tabcount);
        });
      });
    }
  };

})(jQuery);

@msorens
Copy link
Author

msorens commented May 4, 2023

Correction to my initial report: I just went through focussing on semgrep version: turns out this was also a problem in 1.19.0. No problem with 1.18.0.

@IagoAbal
Copy link
Collaborator

IagoAbal commented May 12, 2023

@ocean thanks a lot for this reproduction! In your particular example I could point to r/javascript.express.security.injection.raw-html-format.raw-html-format as the rule that triggered the error.

@IagoAbal
Copy link
Collaborator

I suspect this problem was introduced by 8588460 ("feat(taint): multiple source traces (#7291)").

@IagoAbal IagoAbal self-assigned this May 12, 2023
IagoAbal added a commit that referenced this issue May 12, 2023
If the `mvalue`s being compared happen to contain `stmt`s, then this
will raise a fatal error:

    Failure "Call AST_utils.with_xxx_equal to avoid this error."

We already had a use of `Metavariable.equal_mvalue` previously but this
did not cause much trouble because it was only used if option
`taint_unify_mvars` was set. But then we started using `equal_mvalue`
for essentially all taint-labels rules at the same time as SR was
adding more taint-labels rules... making this fatal error more likely.

Replaced `equal_mvalue` with `Matching_generic.equal_ast_bound_code`
given that it's actually what we use when unifying metavariables.

Also added a Semgrep rule to prevent this in the future.

Fixes: 8588460 ("feat(taint): multiple source traces (#7291)")

Closes #7694

test plan:
make test # test added
IagoAbal added a commit that referenced this issue May 12, 2023
If the `mvalue`s being compared happen to contain `stmt`s, then this
will raise a fatal error:

    Failure "Call AST_utils.with_xxx_equal to avoid this error."

We already had a use of `Metavariable.equal_mvalue` previously but this
did not cause much trouble because it was only used if option
`taint_unify_mvars` was set. But then we started using `equal_mvalue`
for essentially all taint-labels rules at the same time as SR was
adding more taint-labels rules... making this fatal error more likely.

Replaced `equal_mvalue` with `Matching_generic.equal_ast_bound_code`
given that it's actually what we use when unifying metavariables.

Also added a Semgrep rule to prevent this in the future.

Fixes: 8588460 ("feat(taint): multiple source traces (#7291)")

Closes #7694

test plan:
make test # test added
IagoAbal added a commit that referenced this issue May 12, 2023
If the `mvalue`s being compared happen to contain `stmt`s, then this
will raise a fatal error:

    Failure "Call AST_utils.with_xxx_equal to avoid this error."

We already had a use of `Metavariable.equal_mvalue` previously but this
did not cause much trouble because it was only used if option
`taint_unify_mvars` was set. But then we started using `equal_mvalue`
for essentially all taint-labels rules at the same time as SR was adding
more taint-labels rules... making this fatal error more likely.

Replaced `equal_mvalue` with `Matching_generic.equal_ast_bound_code`
given that it's actually what we use when unifying metavariables.

Also added a Semgrep rule to prevent this in the future.

Fixes: 8588460 ("feat(taint): multiple source traces (#7291)")

Closes #7694

test plan:
make test # test added

PR checklist:

- [x] Purpose of the code is [evident to future
readers](https://semgrep.dev/docs/contributing/contributing-code/#explaining-code)
- [x] Tests included or PR comment includes a reproducible test plan
- [x] Documentation is up-to-date
- [x] A changelog entry was [added to
changelog.d](https://semgrep.dev/docs/contributing/contributing-code/#adding-a-changelog-entry)
for any user-facing change
- [x] Change has no security implications (otherwise, ping security
team)

If you're unsure about any of this, please see:

- [Contribution
guidelines](https://semgrep.dev/docs/contributing/contributing-code)!
- [One of the more specific guides located
here](https://semgrep.dev/docs/contributing/contributing/)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

6 participants
@ocean @mjambon @ievans @IagoAbal @msorens and others