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

Remove incorrect fix for cookie-issecure-false.yaml #3236

Closed
wants to merge 1 commit into from

Conversation

0xDC0DE
Copy link
Contributor

@0xDC0DE 0xDC0DE commented Nov 30, 2023

This fix is seemingly copied from another rule. This rule checks for missing setSecure(true), but the fix replaces a setSecure(false) with a setSecure(true).

For a good autofix, this rule needs to be split into two rules:

  • One for a missing setSecure call, that adds one with the right arguments
  • One for a setSecure call with false that needs to be replaced with true.

Neither will need fix-regex, but can be fixed with a regular fix.

This fix is seemingly copied from another rule. This rule checks for missing `setSecure(true)`, but the fix replaces a `setSecure(false)` with a `setSecure(true)`.

For a good autofix, this rule needs to be split into two rules:
- One for a missing `setSecure` call, that adds one with the right arguments
- One for a `setSecure` call with `false` that needs to be replaced with `true`.

Neither will need `fix-regex`, but can be fixed with a regular `fix`.
@inkz
Copy link
Member

inkz commented Dec 25, 2023

@0xDC0DE do you think this update is relevant? https://github.com/semgrep/semgrep-rules/blob/develop/java/servlets/security/cookie-issecure-false.yaml#L13
or it is better to remove fix anyway?

@0xDC0DE
Copy link
Contributor Author

0xDC0DE commented Jan 8, 2024

No that fix seems fine.

@0xDC0DE 0xDC0DE closed this Jan 8, 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