Skip to content

Fix memory allocation in PSOperator and Type1FontProgram #703

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.

Sign up for GitHub

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

Open
MaximPlusov wants to merge 1 commit into
base: integration
Choose a base branch
from
Open

Fix memory allocation in PSOperator and Type1FontProgram #703

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/main/java/org/verapdf/parser/postscript/PSOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
* @author Sergey Shemyakov
*/
public class PSOperator extends PSObject {
private static final int MAX_PS_ARRAY_SIZE = 1 << 16;
private static final int MAX_PS_FOR_ITERATIONS = 10_000;
private Stack<COSObject> operandStack;
private Map<ASAtom, COSObject> userDict;
private final String operator;
Expand Down Expand Up @@ -536,6 +538,9 @@ private void load() throws PostScriptException {
private void array() throws PostScriptException {
try {
int arraySize = getTopNumber().getInteger().intValue();
if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) {
throw new PostScriptException("Array size " + arraySize + " is out of range");
}
Comment on lines 540 to +543
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Validate array size before narrowing to int.

Line 540 narrows to int before range validation, so oversized integers can wrap and evade the intended guard.

Proposed fix 
-            int arraySize = getTopNumber().getInteger().intValue();
-            if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) {
-                throw new PostScriptException("Array size " + arraySize + " is out of range");
+            long requestedSize = getTopNumber().getInteger();
+            if (requestedSize < 0 || requestedSize > MAX_PS_ARRAY_SIZE) {
+                throw new PostScriptException("Array size " + requestedSize + " is out of range");
             }
+            int arraySize = Math.toIntExact(requestedSize);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
int arraySize = getTopNumber().getInteger().intValue();
if (arraySize < 0 || arraySize > MAX_PS_ARRAY_SIZE) {
throw new PostScriptException("Array size " + arraySize + " is out of range");
}
long requestedSize = getTopNumber().getInteger();
if (requestedSize < 0 || requestedSize > MAX_PS_ARRAY_SIZE) {
throw new PostScriptException("Array size " + requestedSize + " is out of range");
}
int arraySize = Math.toIntExact(requestedSize);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/org/verapdf/parser/postscript/PSOperator.java` around lines 540
- 543, The code narrows the value from getTopNumber().getInteger() to an int
(arraySize) before checking range, allowing very large integers to wrap; change
the validation to check the BigInteger/long magnitude against MAX_PS_ARRAY_SIZE
and non-negativity before converting to int (i.e., obtain the BigInteger/Number
from getTopNumber().getInteger(), compare it to
BigInteger.valueOf(MAX_PS_ARRAY_SIZE) and zero, and only then call
intValueExact()/intValue() to assign to arraySize or throw PostScriptException
if out of range), updating any surrounding logic in the method in PSOperator
where arraySize is used.

COSObject array = COSArray.construct(arraySize);
for (int i = 0; i < arraySize; i++) {
array.add(COSObject.getEmpty());
Expand Down Expand Up @@ -577,11 +582,17 @@ private void opFor() throws PostScriptException {
if (!(proc instanceof PSProcedure)) {
throw new PostScriptException("Object is not a procedure");
}
COSObject limit = getTopNumber();
COSObject increment = getTopNumber();
COSObject initial = getTopNumber();
for (long i = initial.getInteger(); i <= limit.getInteger();
i += increment.getInteger()) {
long limit = getTopNumber().getInteger();
long increment = getTopNumber().getInteger();
long initial = getTopNumber().getInteger();
if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
throw new PostScriptException("Wrong increment value " + increment);
}
Comment on lines +588 to +590
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don’t throw on non-entering for loops; return zero iterations instead.

The direction-mismatch cases should be valid no-op loops, not PostScriptException. Throwing here breaks expected for semantics.

Proposed fix 
-            if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
+            if (increment == 0) {
                 throw new PostScriptException("Wrong increment value " + increment);
             }
+            if ((increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
+                return;
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (increment == 0 || (increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
throw new PostScriptException("Wrong increment value " + increment);
}
if (increment == 0) {
throw new PostScriptException("Wrong increment value " + increment);
}
if ((increment > 0 && initial > limit) || (increment < 0 && initial < limit)) {
return;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/org/verapdf/parser/postscript/PSOperator.java` around lines 588
- 590, In PSOperator (the block checking increment/initial/limit where it
currently does "if (increment == 0 || (increment > 0 && initial > limit) ||
(increment < 0 && initial < limit)) { throw new PostScriptException(...); }"),
change the behavior so only a zero increment throws PostScriptException; remove
the throw for direction-mismatch cases and instead treat them as no-op loops by
returning zero iterations (or setting the loop count/iterator to 0) when
(increment > 0 && initial > limit) or (increment < 0 && initial < limit); keep
the check and throw for increment == 0, and ensure you return the appropriate
zero-iteration value from the surrounding method so callers handle the no-op
correctly.

long iterations = Math.abs((limit - initial) / increment) + 1;
if (iterations > MAX_PS_FOR_ITERATIONS) {
throw new PostScriptException("Loop iteration count " + iterations + " is too large");
}
for (long i = initial; (increment > 0 ? i <= limit : i >= limit); i += increment) {
operandStack.push(COSInteger.construct(i));
((PSProcedure) proc).executeProcedure(operandStack, userDict);
}
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/org/verapdf/pd/font/type1/Type1FontProgram.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class Type1FontProgram extends PSParser implements FontProgram {
public static final Logger LOGGER =
Logger.getLogger(Type1FontProgram.class.getCanonicalName());
static final double[] DEFAULT_FONT_MATRIX = {0.001, 0, 0, 0.001, 0, 0};
private static final int MAX_TO_EXECUTE_DEPTH = 64;

private final COSKey key;

Expand Down Expand Up @@ -187,12 +188,19 @@ private void processObject(COSObject nextObject) throws IOException, PostScriptE
}

private void toExecute(COSObject next) throws PostScriptException {
toExecute(next, 0);
}

private void toExecute(COSObject next, int depth) throws PostScriptException {
if (depth > MAX_TO_EXECUTE_DEPTH) {
throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth");
Comment on lines +195 to +196
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 19, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix off-by-one in recursion depth guard.

depth > MAX_TO_EXECUTE_DEPTH permits one extra recursive level beyond the stated max. If the cap is intended to be 64, this should fail on equality too.

Suggested patch 
-        if (depth > MAX_TO_EXECUTE_DEPTH) {
+        if (depth >= MAX_TO_EXECUTE_DEPTH) {
             throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth");
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (depth > MAX_TO_EXECUTE_DEPTH) {
throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth");
if (depth >= MAX_TO_EXECUTE_DEPTH) {
throw new PostScriptException("Type 1 font program exceeded toExecute recursion depth");
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/org/verapdf/pd/font/type1/Type1FontProgram.java` around lines
195 - 196, In Type1FontProgram change the recursion guard so it throws when
depth is greater than or equal to the limit: replace the current condition using
depth and MAX_TO_EXECUTE_DEPTH (currently "depth > MAX_TO_EXECUTE_DEPTH") with a
check that fails on equality as well (e.g., "depth >= MAX_TO_EXECUTE_DEPTH") so
the PostScriptException in the toExecute recursion path is raised at the
intended maximum; update the clause that throws PostScriptException("Type 1 font
program exceeded toExecute recursion depth") accordingly.

}
PSObject operator = PSObject.getPSObject(next);
if (operator instanceof PSOperator) {
if (!OPERATORS_KEYWORDS.contains(((PSOperator) operator).getOperator())) {
COSObject dictEntry = userDict.get(ASAtom.getASAtom(((PSOperator) operator).getOperator()));
if (dictEntry != null) {
toExecute(dictEntry);
toExecute(dictEntry, depth + 1);
}
} else {
operator.execute(operandStack, userDict);
Expand Down
Loading