Skip to content

add crash report test for the receiver#30

Merged
rochdev merged 26 commits intomainfrom
rochdev/test-crash-report
Sep 17, 2024
Merged

add crash report test for the receiver#30
rochdev merged 26 commits intomainfrom
rochdev/test-crash-report

Conversation

@rochdev
Copy link
Copy Markdown
Member

@rochdev rochdev commented Sep 11, 2024

This PR adds a test that the crashtracker is actually sending crash reports and works around a bug in the receiver that was making it crash but that crash was hidden from the Node.js binding so it wasn't detected in the initialization test.

It's worth noting that this is a workaround and the receiver should be properly fixed, but in the meantime it allows the feature to work. There were also some problems with Alpine so I removed it from the prebuilds for now.

execSync('npm install --silent', { cwd })
execSync('npm run --silent build', { cwd })

const app = express()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Security misconfiguration detected (...read more)

Improve your overall server security by taking the step to reduce the likelihood of server fingerprinting the software being used on the server.

By default, Express.js sends the X-Powered-By response header banner which can be disabled with app.disable('X-Powered-By').

If you're using helmet, you can use either of these methods too:

  • app.use(hidePoweredBy())
  • app.use(helmet.hidePoweredBy())

Learn More

View in Datadog  Leave us feedback  Documentation

execSync('npm install --silent', { cwd })
execSync('npm run --silent build', { cwd })

const app = express()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Security misconfiguration detected (helmet missing) (...read more)

Per Express documentation:

Helmet can help protect your app from some well-known web vulnerabilities by setting HTTP headers appropriately.

This rule will check whether you've set app.use(helmet()) within the file that you've called express()

View in Datadog  Leave us feedback  Documentation

Comment on lines +270 to +292
"node_modules/express/node_modules/body-parser": {
"version": "1.20.2",
"resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.2.tgz",
"integrity": "sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA==",
"dependencies": {
"bytes": "3.1.2",
"content-type": "~1.0.5",
"debug": "2.6.9",
"depd": "2.0.0",
"destroy": "1.2.0",
"http-errors": "2.0.0",
"iconv-lite": "0.4.24",
"on-finished": "2.4.1",
"qs": "6.11.0",
"raw-body": "2.5.2",
"type-is": "~1.6.18",
"unpipe": "1.0.0"
},
"engines": {
"node": ">= 0.8",
"npm": "1.2.8000 || >= 1.4.16"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Library Vulnerability

body-parser → 1.20.2

body-parser vulnerable to denial of service when url encoding is enabled (...read more)

Impact

body-parser <1.20.3 is vulnerable to denial of service when url encoding is enabled. A malicious actor using a specially crafted payload could flood the server with a large number of requests, resulting in denial of service.

Patches

this issue is patched in 1.20.3

References

View in Datadog  Leave us feedback  Documentation

Comment on lines +645 to +658
"node_modules/serve-static": {
"version": "1.15.0",
"resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.15.0.tgz",
"integrity": "sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g==",
"dependencies": {
"encodeurl": "~1.0.2",
"escape-html": "~1.0.3",
"parseurl": "~1.3.3",
"send": "0.18.0"
},
"engines": {
"node": ">= 0.8.0"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

serve-static → 1.15.0

serve-static vulnerable to template injection that can lead to XSS (...read more)

Impact

passing untrusted user input - even after sanitizing it - to redirect() may execute untrusted code

Patches

this issue is patched in serve-static 1.16.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

},
"dependencies": {
"body-parser": "^1.20.3",
"express": "^4.19.2"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

express → 4.19.2

express vulnerable to XSS via response.redirect() (...read more)

Impact

In express <4.20.0, passing untrusted user input - even after sanitizing it - to response.redirect() may execute untrusted code

Patches

this issue is patched in express 4.20.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

Comment on lines +540 to +544
"node_modules/path-to-regexp": {
"version": "0.1.7",
"resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.7.tgz",
"integrity": "sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ=="
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Library Vulnerability

path-to-regexp → 0.1.7

path-to-regexp outputs backtracking regular expressions (...read more)

Impact

A bad regular expression is generated any time you have two parameters within a single segment, separated by something that is not a period (.). For example, /:a-:b.

Patches

For users of 0.1, upgrade to 0.1.10. All other users should upgrade to 8.0.0.

These versions add backtrack protection when a custom regex pattern is not provided:

They do not protect against vulnerable user supplied capture groups. Protecting against explicit user patterns is out of scope for this library and not considered a vulnerability.

Version 7.1.0 can enable strict: true and get an error when the regular expression might be bad.

Version 8.0.0 removes the features that can cause a ReDoS.

Workarounds

All versions can be patched by providing a custom regular expression for parameters after the first in a single segment. As long as the custom regular expression does not match the text before the parameter, you will be safe. For example, change /:a-:b to /:a-:b([^-/]+).

If paths cannot be rewritten and versions cannot be upgraded, another alternative is to limit the URL length. For example, halving the attack string improves performance by 4x faster.

Details

Using /:a-:b will produce the regular expression /^\/([^\/]+?)-([^\/]+?)\/?$/. This can be exploited by a path such as /a${'-a'.repeat(8_000)}/a. OWASP has a good example of why this occurs, but the TL;DR is the /a at the end ensures this route would never match but due to naive backtracking it will still attempt every combination of the :a-:b on the repeated 8,000 -a.

Because JavaScript is single threaded and regex matching runs on the main thread, poor performance will block the event loop and can lead to a DoS. In local benchmarks, exploiting the unsafe regex will result in performance that is over 1000x worse than the safe regex. In a more realistic environment using Express v4 and 10 concurrent connections, this translated to average latency of ~600ms vs 1ms.

References

View in Datadog  Leave us feedback  Documentation

Comment on lines +617 to +639
"node_modules/send": {
"version": "0.18.0",
"resolved": "https://registry.npmjs.org/send/-/send-0.18.0.tgz",
"integrity": "sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg==",
"dependencies": {
"debug": "2.6.9",
"depd": "2.0.0",
"destroy": "1.2.0",
"encodeurl": "~1.0.2",
"escape-html": "~1.0.3",
"etag": "~1.8.1",
"fresh": "0.5.2",
"http-errors": "2.0.0",
"mime": "1.6.0",
"ms": "2.1.3",
"on-finished": "2.4.1",
"range-parser": "~1.2.1",
"statuses": "2.0.1"
},
"engines": {
"node": ">= 0.8.0"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

send → 0.18.0

send vulnerable to template injection that can lead to XSS (...read more)

Impact

passing untrusted user input - even after sanitizing it - to SendStream.redirect() may execute untrusted code

Patches

this issue is patched in send 0.19.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

execSync('npm install --silent', { cwd })
execSync('npm run --silent build', { cwd })

const app = express()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Security misconfiguration detected (...read more)

Improve your overall server security by taking the step to reduce the likelihood of server fingerprinting the software being used on the server.

By default, Express.js sends the X-Powered-By response header banner which can be disabled with app.disable('X-Powered-By').

If you're using helmet, you can use either of these methods too:

  • app.use(hidePoweredBy())
  • app.use(helmet.hidePoweredBy())

Learn More

View in Datadog  Leave us feedback  Documentation

execSync('npm install --silent', { cwd })
execSync('npm run --silent build', { cwd })

const app = express()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Code Vulnerability

Security misconfiguration detected (helmet missing) (...read more)

Per Express documentation:

Helmet can help protect your app from some well-known web vulnerabilities by setting HTTP headers appropriately.

This rule will check whether you've set app.use(helmet()) within the file that you've called express()

View in Datadog  Leave us feedback  Documentation

Comment on lines +270 to +292
"node_modules/express/node_modules/body-parser": {
"version": "1.20.2",
"resolved": "https://registry.npmjs.org/body-parser/-/body-parser-1.20.2.tgz",
"integrity": "sha512-ml9pReCu3M61kGlqoTm2umSXTlRTuGTx0bfYj+uIUKKYycG5NtSbeetV3faSU6R7ajOPw0g/J1PvK4qNy7s5bA==",
"dependencies": {
"bytes": "3.1.2",
"content-type": "~1.0.5",
"debug": "2.6.9",
"depd": "2.0.0",
"destroy": "1.2.0",
"http-errors": "2.0.0",
"iconv-lite": "0.4.24",
"on-finished": "2.4.1",
"qs": "6.11.0",
"raw-body": "2.5.2",
"type-is": "~1.6.18",
"unpipe": "1.0.0"
},
"engines": {
"node": ">= 0.8",
"npm": "1.2.8000 || >= 1.4.16"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Library Vulnerability

body-parser → 1.20.2

body-parser vulnerable to denial of service when url encoding is enabled (...read more)

Impact

body-parser <1.20.3 is vulnerable to denial of service when url encoding is enabled. A malicious actor using a specially crafted payload could flood the server with a large number of requests, resulting in denial of service.

Patches

this issue is patched in 1.20.3

References

View in Datadog  Leave us feedback  Documentation

Comment on lines +645 to +658
"node_modules/serve-static": {
"version": "1.15.0",
"resolved": "https://registry.npmjs.org/serve-static/-/serve-static-1.15.0.tgz",
"integrity": "sha512-XGuRDNjXUijsUL0vl6nSD7cwURuzEgglbOaFuZM9g3kwDXOWVTck0jLzjPzGD+TazWbboZYu52/9/XPdUgne9g==",
"dependencies": {
"encodeurl": "~1.0.2",
"escape-html": "~1.0.3",
"parseurl": "~1.3.3",
"send": "0.18.0"
},
"engines": {
"node": ">= 0.8.0"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

serve-static → 1.15.0

serve-static vulnerable to template injection that can lead to XSS (...read more)

Impact

passing untrusted user input - even after sanitizing it - to redirect() may execute untrusted code

Patches

this issue is patched in serve-static 1.16.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

},
"dependencies": {
"body-parser": "^1.20.3",
"express": "^4.19.2"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

express → 4.19.2

express vulnerable to XSS via response.redirect() (...read more)

Impact

In express <4.20.0, passing untrusted user input - even after sanitizing it - to response.redirect() may execute untrusted code

Patches

this issue is patched in express 4.20.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

Comment on lines +540 to +544
"node_modules/path-to-regexp": {
"version": "0.1.7",
"resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.7.tgz",
"integrity": "sha512-5DFkuoqlv1uYQKxy8omFBeJPQcdoE07Kv2sferDCrAq1ohOU+MSDswDIbnx3YAM60qIOnYa53wBhXW0EbMonrQ=="
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Library Vulnerability

path-to-regexp → 0.1.7

path-to-regexp outputs backtracking regular expressions (...read more)

Impact

A bad regular expression is generated any time you have two parameters within a single segment, separated by something that is not a period (.). For example, /:a-:b.

Patches

For users of 0.1, upgrade to 0.1.10. All other users should upgrade to 8.0.0.

These versions add backtrack protection when a custom regex pattern is not provided:

They do not protect against vulnerable user supplied capture groups. Protecting against explicit user patterns is out of scope for this library and not considered a vulnerability.

Version 7.1.0 can enable strict: true and get an error when the regular expression might be bad.

Version 8.0.0 removes the features that can cause a ReDoS.

Workarounds

All versions can be patched by providing a custom regular expression for parameters after the first in a single segment. As long as the custom regular expression does not match the text before the parameter, you will be safe. For example, change /:a-:b to /:a-:b([^-/]+).

If paths cannot be rewritten and versions cannot be upgraded, another alternative is to limit the URL length. For example, halving the attack string improves performance by 4x faster.

Details

Using /:a-:b will produce the regular expression /^\/([^\/]+?)-([^\/]+?)\/?$/. This can be exploited by a path such as /a${'-a'.repeat(8_000)}/a. OWASP has a good example of why this occurs, but the TL;DR is the /a at the end ensures this route would never match but due to naive backtracking it will still attempt every combination of the :a-:b on the repeated 8,000 -a.

Because JavaScript is single threaded and regex matching runs on the main thread, poor performance will block the event loop and can lead to a DoS. In local benchmarks, exploiting the unsafe regex will result in performance that is over 1000x worse than the safe regex. In a more realistic environment using Express v4 and 10 concurrent connections, this translated to average latency of ~600ms vs 1ms.

References

View in Datadog  Leave us feedback  Documentation

Comment on lines +617 to +639
"node_modules/send": {
"version": "0.18.0",
"resolved": "https://registry.npmjs.org/send/-/send-0.18.0.tgz",
"integrity": "sha512-qqWzuOjSFOuqPjFe4NOsMLafToQQwBSOEpS+FwEt3A2V3vKubTquT3vmLTQpFgMXp8AlFWFuP1qKaJZOtPpVXg==",
"dependencies": {
"debug": "2.6.9",
"depd": "2.0.0",
"destroy": "1.2.0",
"encodeurl": "~1.0.2",
"escape-html": "~1.0.3",
"etag": "~1.8.1",
"fresh": "0.5.2",
"http-errors": "2.0.0",
"mime": "1.6.0",
"ms": "2.1.3",
"on-finished": "2.4.1",
"range-parser": "~1.2.1",
"statuses": "2.0.1"
},
"engines": {
"node": ">= 0.8.0"
}
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Library Vulnerability

send → 0.18.0

send vulnerable to template injection that can lead to XSS (...read more)

Impact

passing untrusted user input - even after sanitizing it - to SendStream.redirect() may execute untrusted code

Patches

this issue is patched in send 0.19.0

Workarounds

users are encouraged to upgrade to the patched version of express, but otherwise can workaround this issue by making sure any untrusted inputs are safe, ideally by validating them against an explicit allowlist

Details

successful exploitation of this vector requires the following:

  1. The attacker MUST control the input to response.redirect()
  2. express MUST NOT redirect before the template appears
  3. the browser MUST NOT complete redirection before:
  4. the user MUST click on the link in the template

View in Datadog  Leave us feedback  Documentation

Comment thread test/crashtracker/index.js Outdated
}
})
} catch (e) {
console.log(e)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 Code Quality Violation

Unexpected console statement. (...read more)

Debugging with console (such as console.log or console.info) is not considered a bad practice, but these statements can be accidentally left in production code, leading to unnecessary log pollution. It is important to remove or replace these debugging statements to maintain clean and secure production builds.

View in Datadog  Leave us feedback  Documentation

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 13, 2024

Overall package size

Self size: 19.84 MB
Deduped: 19.84 MB
No deduping: 19.84 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------|

🤖 This report was automatically generated by heaviest-objects-in-the-universe

@rochdev rochdev marked this pull request as ready for review September 13, 2024 23:23
@rochdev rochdev marked this pull request as draft September 17, 2024 19:37
@rochdev rochdev force-pushed the rochdev/test-crash-report branch 3 times, most recently from c6f6ccb to b6d3644 Compare September 17, 2024 21:34
@rochdev rochdev marked this pull request as ready for review September 17, 2024 22:04
@rochdev rochdev merged commit 5ff0387 into main Sep 17, 2024
@rochdev rochdev deleted the rochdev/test-crash-report branch September 17, 2024 22:20
rochdev added a commit that referenced this pull request Sep 17, 2024
rochdev added a commit that referenced this pull request Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants