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

Address CodeQL security alerts #6609

Merged
merged 3 commits into from
Dec 5, 2023
Merged

Address CodeQL security alerts #6609

merged 3 commits into from
Dec 5, 2023

Conversation

lildude
Copy link
Member

@lildude lildude commented Nov 9, 2023

Description

CodeQL has been enabled on this repo and the analysis has revealed two issues repeated a few times that this PR addresses:

  1. Polynomial regular expression used on uncontrolled data
  2. Use of Kernel.open or IO.read or similar sinks with a non-constant value

@@ -795,7 +795,7 @@ def generated_sorbet_rbi?
return false unless lines.count >= 5
lines[0].match?(/^# typed:/) &&
lines[2].include?("DO NOT EDIT MANUALLY") &&
lines[4].match?(/^# Please.*run.*`.*tapioca/)
lines[4].match?(/^# Please (run|instead update this file by running) `bin\/tapioca/)
Copy link
Member Author

Choose a reason for hiding this comment

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

From the alert:

"This regular expression that depends on a library input may run slow on strings starting with '# Pleaserun' and with many repetitions of 'run'."

return lines[0].match(/\/\* GIMP [a-zA-Z0-9\- ]+ C\-Source image dump \(.+?\.c\) \*\//) ||
lines[0].match(/\/\* GIMP header image file format \([a-zA-Z0-9\- ]+\)\: .+?\.h \*\//)
return lines[0].match(/^\/\* GIMP [a-zA-Z0-9\- ]+ C\-Source image dump \(.+?\.c\) \*\//) ||
lines[0].match(/^\/\* GIMP header image file format \([a-zA-Z0-9\- ]+\)\: .+?\.h \*\//)
Copy link
Member Author

Choose a reason for hiding this comment

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

From the alert:

"This regular expression that depends on a library input may run slow on strings starting with '/* GIMP header image file format ( ): ' and with many repetitions of '/* GIMP header image file format ( ): '.

@@ -693,8 +693,8 @@ def generated_gamemakerstudio?
def generated_gimp?
return false unless ['.c', '.h'].include? extname
return false unless lines.count > 0
return lines[0].match(/\/\* GIMP [a-zA-Z0-9\- ]+ C\-Source image dump \(.+?\.c\) \*\//) ||
lines[0].match(/\/\* GIMP header image file format \([a-zA-Z0-9\- ]+\)\: .+?\.h \*\//)
return lines[0].match(/^\/\* GIMP [a-zA-Z0-9\- ]+ C\-Source image dump \(.+?\.c\) \*\//) ||
Copy link
Member Author

Choose a reason for hiding this comment

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

From the alert:

"This regular expression that depends on a library input may run slow on strings starting with '/* GIMP C-Source image dump (' and with many repetitions of '/* GIMP C-Source image dump ('."

@@ -301,7 +301,7 @@ def generated_postscript?

# Type 1 and Type 42 fonts converted to PostScript are stored as hex-encoded byte streams; these
# streams are always preceded the `eexec` operator (if Type 1), or the `/sfnts` key (if Type 42).
return true if data =~ /(\n|\r\n|\r)\s*(?:currentfile eexec\s+|\/sfnts\s+\[\1<)\h{8,}\1/
return true if data =~ /^\s*(?:currentfile eexec\s+|\/sfnts\s+\[\s<)/
Copy link
Member Author

@lildude lildude Nov 9, 2023

Choose a reason for hiding this comment

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

From the alert:

"This regular expression that depends on a library input may run slow on strings starting with '\n' and with many repetitions of '\n'."

This seems the most silly to me, but I also realised this is probably more complex than it really needs to be so I've simplified it. It was also Ruby-specific.

@lildude lildude marked this pull request as ready for review November 9, 2023 18:05
@lildude lildude requested a review from a team as a code owner November 9, 2023 18:05
@lildude lildude requested a review from Alhadis November 9, 2023 18:05
Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

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

Smart catches.

@lildude lildude disabled auto-merge December 5, 2023 09:46
@lildude lildude merged commit 2a527e1 into master Dec 5, 2023
8 checks passed
@lildude lildude deleted the lildude/codeql-suggestions branch December 5, 2023 09:46
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants