Skip to content

Conversation

@gkjung
Copy link
Contributor

@gkjung gkjung commented Jun 15, 2022

How to message originally looked:
jquery 3.1.0 has known vulnerabilities:\nseverity: medium; summary: jQuery before 3.4.0, as used in Drupal, Backdrop CMS, and other products, mishandles jQuery.extend(true, {}, ...) because of Object.prototype pollution; CVE: CVE-2019-11358; https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/ https://nvd.nist.gov/vuln/detail/CVE-2019-11358 https://github.com/jquery/jquery/commit/753d591aea698e57d6db58c9f722cd0808619b1b\nseverity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11022; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/\nseverity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11023; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/

New format to remove \n:
jquery 3.1.0 has known vulnerabilities: severity: medium; summary: jQuery before 3.4.0, as used in Drupal, Backdrop CMS, and other products, mishandles jQuery.extend(true, {}, ...) because of Object.prototype pollution; CVE: CVE-2019-11358; https://blog.jquery.com/2019/04/10/jquery-3-4-0-released/ https://nvd.nist.gov/vuln/detail/CVE-2019-11358 https://github.com/jquery/jquery/commit/753d591aea698e57d6db58c9f722cd0808619b1b; severity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11022; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/; severity: medium; summary: Regex in its jQuery.htmlPrefilter sometimes may introduce XSS; CVE: CVE-2020-11023; https://blog.jquery.com/2020/04/10/jquery-3-5-0-released/

export class RuleResultRecombinator {

public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set<string>): Promise<RecombinedRuleResults> {
public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set<string>, verboseViolations = false): Promise<RecombinedRuleResults> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it okay to use a default param like this to avoid affecting the method signature everywhere else?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say so.

export class RuleResultRecombinator {

public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set<string>): Promise<RecombinedRuleResults> {
public static async recombineAndReformatResults(results: RuleResult[], format: OUTPUT_FORMAT, executedEngines: Set<string>, verboseViolations = false): Promise<RecombinedRuleResults> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say so.

// in the json format we need to replace new lines in the message
// for the first line (ending with a colon) we will replace it with a space
// for following lines, we will replace it with a semicolon and a space
violation.message = violation.message.replace(/:\n/g, ': ').replace(/\n/g, '; ');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may wish to check what that does to non-RetireJS violations. I.e., scan something with both retirejs and eslint violations, and use verbose violations to see what the output looks like.
It's unlikely to cause any problems, but it's still probably worth taking a quick check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I am going to add a check so this only happens when the engine is retire-js, just to be safe.


if (verboseViolations) {
for (const result of results) {
if (result.engine === 'retire-js') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use retire-js as a hardcoded string.
Add this import:
import {ENGINE} from '../../Constants'; and then change this line to if (result.engine === ENGINE.RETIRE_JS) {`

@gkjung gkjung force-pushed the d/W-11267235 branch 2 times, most recently from 0ee1c9b to 5ec6b5a Compare June 16, 2022 19:51
@gkjung gkjung merged commit c4018c5 into dev-3 Jun 16, 2022
@jfeingold35 jfeingold35 deleted the d/W-11267235 branch June 23, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants