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

Hardening suggestions for Stirling-PDF / certValidate #2395

Merged
merged 2 commits into from
Dec 5, 2024

Conversation

pixeebot[bot]
Copy link
Contributor

@pixeebot pixeebot bot commented Dec 5, 2024

I've reviewed the recently opened PR (2394 - PDF Cert validation) and have identified some area(s) that could benefit from additional hardening measures.

These changes should help prevent potential security vulnerabilities and improve overall code quality.

Thank you for your consideration!
🧚🤖 Powered by Pixeebot

Feedback | Community | Docs

@pixeebot pixeebot bot requested a review from Frooodle as a code owner December 5, 2024 13:59
@pixeebot pixeebot bot requested a review from Frooodle December 5, 2024 13:59
@@ -44,14 +45,14 @@ private void loadMozillaCertificates() throws Exception {
boolean inCert = false;
int certCount = 0;

while ((line = reader.readLine()) != null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replaced with a call that offers an upper bound on the number of characters that will be read before giving up and throwing a security exception

if (line.startsWith("CKA_VALUE MULTILINE_OCTAL")) {
inCert = true;
certData = new StringBuilder();
continue;
}
if (inCert) {
if (line.equals("END")) {
if ("END".equals(line)) {
inCert = false;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switch order of literals to prevent NullPointerException

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Dec 5, 2024
@pixeebot pixeebot bot force-pushed the pixeebot/certValidate branch from 91fd35f to 558a83f Compare December 5, 2024 13:59
@github-actions github-actions bot added the Java Pull requests that update Java code label Dec 5, 2024
@pixeebot pixeebot bot force-pushed the pixeebot/certValidate branch from 558a83f to 720e57f Compare December 5, 2024 14:00
@Frooodle Frooodle merged commit 6152d3f into certValidate Dec 5, 2024
2 of 3 checks passed
@pixeebot pixeebot bot deleted the pixeebot/certValidate branch December 5, 2024 14:02
Frooodle added a commit that referenced this pull request Dec 5, 2024
* verifyCerts

* cert info

* Hardening suggestions for Stirling-PDF / certValidate (#2395)

* Protect `readLine()` against DoS

* Switch order of literals to prevent NullPointerException

---------

Co-authored-by: pixeebot[bot] <104101892+pixeebot[bot]@users.noreply.github.com>

* some basic html excaping and translation fixing

---------

Co-authored-by: pixeebot[bot] <104101892+pixeebot[bot]@users.noreply.github.com>
Co-authored-by: a <a>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java Pull requests that update Java code size:XS This PR changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant