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

[BUGFIX] Fix for potential security risk related to file include #1261

Merged
merged 3 commits into from
Jun 18, 2024

Conversation

beatrycze-volk
Copy link
Collaborator

"include" statement detected. File manipulations are discouraged. Statement is not a function, no parentheses are required. Concatenating is forbidden.

@sebastian-meyer
Copy link
Member

Maybe I am missing something, but I don't see a critical security issue here. Sure, it's bad form and should be fixed, but it's not security related. There are no user variables involved, so it's not possible to inject malicious code. Or am I wrong?

@beatrycze-volk beatrycze-volk force-pushed the replace-include branch 3 times, most recently from d140e03 to d744442 Compare June 13, 2024 10:59
@beatrycze-volk beatrycze-volk marked this pull request as ready for review June 13, 2024 11:25
@beatrycze-volk
Copy link
Collaborator Author

Maybe I am missing something, but I don't see a critical security issue here. Sure, it's bad form and should be fixed, but it's not security related. There are no user variables involved, so it's not possible to inject malicious code. Or am I wrong?

According to Codacy it is critical security problem: https://app.codacy.com/p/298603/issues/index?resultDataId=131127386721

@beatrycze-volk beatrycze-volk marked this pull request as ready for review June 13, 2024 12:42
@sebastian-meyer sebastian-meyer self-requested a review June 18, 2024 16:46
@sebastian-meyer sebastian-meyer added the 🐛 bug A non-security related bug. label Jun 18, 2024
@sebastian-meyer sebastian-meyer changed the title [BUGFIX] Fix for critical security error related to file include [BUGFIX] Fix for potential security risk related to file include Jun 18, 2024
@sebastian-meyer sebastian-meyer merged commit 5a833a4 into kitodo:master Jun 18, 2024
7 checks passed
@beatrycze-volk beatrycze-volk deleted the replace-include branch June 19, 2024 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug A non-security related bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants