From 4f06f64ee2c81affce2ae2446344e78931a29822 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Sun, 13 Sep 2026 22:55:24 -0300 Subject: [PATCH 1/2] fix: prevent /P permissions overflow from corrupting decryption key PdfNumber#intValue() narrowed the underlying double with a plain (int) cast. Some PDF producers write the /Encrypt dictionary's /P (permissions) entry using its unsigned 32-bit decimal representation instead of the equivalent signed value (e.g. 4294965956 instead of -1340) - both encode the identical 4-byte value, and this is spec-legal. Java's narrowing double-to-int conversion saturates rather than wraps for an out-of-range value, so such a /P value was misread as Integer.MAX_VALUE instead of -1340. That corrupted the encryption key the standard security handler derives, so its own recomputed /U check failed for every password - including a correct, empty one - and PdfReader reported BadPasswordException for a document that was never actually password-protected. Route the cast through long first, which reproduces the expected low-order-32-bit truncation instead of clamping. Add PdfNumberTest covering the truncation directly, and DecryptUnsignedPermissionsTest, a PdfReader-level regression test using a crafted PDF whose /P is written this way with an empty user password. Co-Authored-By: Claude Sonnet 5 --- .../java/org/openpdf/text/pdf/PdfNumber.java | 11 +++- .../org/openpdf/text/pdf/PdfNumberTest.java | 31 ++++++++++ .../DecryptUnsignedPermissionsTest.java | 57 ++++++++++++++++++ .../empty-user-password-unsigned-p-value.pdf | Bin 0 -> 615 bytes 4 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 openpdf-core/src/test/java/org/openpdf/text/pdf/PdfNumberTest.java create mode 100644 openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java create mode 100644 openpdf-core/src/test/resources/permissions/empty-user-password-unsigned-p-value.pdf diff --git a/openpdf-core/src/main/java/org/openpdf/text/pdf/PdfNumber.java b/openpdf-core/src/main/java/org/openpdf/text/pdf/PdfNumber.java index b87784f2a..03b446af6 100644 --- a/openpdf-core/src/main/java/org/openpdf/text/pdf/PdfNumber.java +++ b/openpdf-core/src/main/java/org/openpdf/text/pdf/PdfNumber.java @@ -135,11 +135,20 @@ public PdfNumber(float value) { /** * Returns the primitive int value of this object. + *

+ * Some PDF producers write 32-bit values that are meant to be interpreted as a two's-complement + * signed integer (for example the {@code /P} entry of an encryption dictionary) using their + * equivalent unsigned decimal representation instead of the negative signed value, e.g. + * {@code 4294965956} instead of {@code -1340}. A plain {@code (int) value} narrowing cast on the + * underlying double would clamp such an out-of-range value to + * {@link Integer#MAX_VALUE} rather than wrapping it, silently corrupting values like that. Going + * through long first reproduces the C-style truncation to the low-order 32 bits that + * PDF producers/consumers expect. * * @return The value as int */ public int intValue() { - return (int) value; + return (int) (long) value; } /** diff --git a/openpdf-core/src/test/java/org/openpdf/text/pdf/PdfNumberTest.java b/openpdf-core/src/test/java/org/openpdf/text/pdf/PdfNumberTest.java new file mode 100644 index 000000000..edc9e9f38 --- /dev/null +++ b/openpdf-core/src/test/java/org/openpdf/text/pdf/PdfNumberTest.java @@ -0,0 +1,31 @@ +package org.openpdf.text.pdf; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.Test; + +class PdfNumberTest { + + /** + * Some PDF producers write the encryption dictionary's {@code /P} (permissions) entry using its + * equivalent unsigned 32-bit decimal representation instead of the negative signed value, e.g. + * {@code 4294965956} instead of {@code -1340}. {@link PdfNumber#intValue()} must truncate such a + * value to its low-order 32 bits rather than clamping it to {@link Integer#MAX_VALUE}, otherwise + * the permission bits used to derive the decryption key no longer match the value the producer + * used, and a document with an empty user password is wrongly reported as requiring a password. + */ + @Test + void intValueTruncatesOutOfRangeUnsignedRepresentationInsteadOfClamping() { + assertEquals(-1340, new PdfNumber("4294965956").intValue()); + assertEquals(-1, new PdfNumber("4294967295").intValue()); + assertEquals(Integer.MIN_VALUE, new PdfNumber(String.valueOf(1L << 31)).intValue()); + } + + @Test + void intValueRoundTripsValuesWithinIntRange() { + assertEquals(-1340, new PdfNumber("-1340").intValue()); + assertEquals(0, new PdfNumber("0").intValue()); + assertEquals(Integer.MAX_VALUE, new PdfNumber(String.valueOf(Integer.MAX_VALUE)).intValue()); + assertEquals(Integer.MIN_VALUE, new PdfNumber(String.valueOf(Integer.MIN_VALUE)).intValue()); + } +} diff --git a/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java b/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java new file mode 100644 index 000000000..c52b4e954 --- /dev/null +++ b/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java @@ -0,0 +1,57 @@ +package org.openpdf.text.pdf.encryption; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.openpdf.text.pdf.PdfReader; + +/** + * Regression test for a document that has no user password but is nevertheless reported by + * {@link PdfReader} as requiring one, because its {@code /Encrypt} dictionary writes the {@code /P} + * (permissions) entry using the unsigned 32-bit decimal representation of a negative value (e.g. + * {@code 4294965956} instead of {@code -1340}) - a convention some PDF producers use. + *

+ * {@link org.openpdf.text.pdf.PdfNumber#intValue()} used a plain {@code (int)} narrowing cast on the + * underlying double, which clamps such an out-of-range value to + * {@link Integer#MAX_VALUE} instead of truncating it to its low-order 32 bits. The corrupted + * permission value then fed into the standard security handler's key derivation, producing a user key + * that did not match {@code /U}, so {@link PdfReader} threw a {@link org.openpdf.text.exceptions.BadPasswordException} + * even though the correct (empty) password was supplied. + */ +class DecryptUnsignedPermissionsTest { + + static Field ownerPasswordUsedField; + + static boolean isOwnerPasswordUsed(PdfReader pdfReader) { + try { + return ownerPasswordUsedField.getBoolean(pdfReader); + } catch (IllegalArgumentException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } + + @BeforeAll + static void setUpBeforeClass() throws Exception { + ownerPasswordUsedField = PdfReader.class.getDeclaredField("ownerPasswordUsed"); + ownerPasswordUsedField.setAccessible(true); + } + + @Test + void opensWithEmptyPasswordDespiteUnsignedPermissionsValue() throws IOException { + try (InputStream resource = getClass() + .getResourceAsStream("/permissions/empty-user-password-unsigned-p-value.pdf")) { + try (PdfReader pdfReader = new PdfReader(resource)) { + assertTrue(pdfReader.isEncrypted(), "PdfReader fails to report test file to be encrypted."); + assertFalse(isOwnerPasswordUsed(pdfReader), "PdfReader fails to report limited permissions."); + assertEquals(1, pdfReader.getNumberOfPages(), + "PdfReader fails to report the correct number of pages"); + } + } + } +} diff --git a/openpdf-core/src/test/resources/permissions/empty-user-password-unsigned-p-value.pdf b/openpdf-core/src/test/resources/permissions/empty-user-password-unsigned-p-value.pdf new file mode 100644 index 0000000000000000000000000000000000000000..d528d8b2cde6819094b8d57ad8d805bd24145baa GIT binary patch literal 615 zcmah`%Wm6147}?rc=0ja7xx7SIy8xaqAg-LX)i_&``|h%3@MORAnDh4WuQusOL^mP zhTw2EyPNHm_=JrB4rBb)w(Q~cnb>uh!)ZLST{zMiWF79ij)q(*&Cm@@lD z!*PBBAsxGeHl-x9Lsxk5!4?-%Fg`^}w%EiCI+L7LMJmCgM-SJ7#0g)uR!j_m-Y zM{nv0hV4PIAJ7U(i3ekiw?1)H6iw2}p>aGBuZOx=kTfA#?E(seoY&rzQUVv;b?hg| z)yvC7rKuhpz7`m*m4*Uun1{<0hBr*tPc8Kh6TF7@5i|8$|FC%q{fVZU>3#o)K(qZg z)?Aj5*zJ&}*XJ3Q=kD2;n?oy&@@O?tl$5Ox7b=1NCDK)g_Jcnt%_*BL< Ko6YUrw)qQ-l%xUx literal 0 HcmV?d00001 From fa72846bb1d3b8794cb8ea4f7f814470ffd41e14 Mon Sep 17 00:00:00 2001 From: Diego Garcia Date: Mon, 14 Sep 2026 19:10:53 -0300 Subject: [PATCH 2/2] test: use PdfReader.isOwnerPasswordUsed() instead of reflection DecryptUnsignedPermissionsTest reached PdfReader's private ownerPasswordUsed field via reflection (setAccessible), which Codacy flagged as a high-severity visibility-alteration issue, plus a medium issue for wrapping the reflection exceptions in a raw RuntimeException. PdfReader already exposes a public isOwnerPasswordUsed() getter that returns the same field, so the reflection is unnecessary. Calling it directly returns the identical value and removes both findings. Co-Authored-By: Claude Sonnet 5 --- .../DecryptUnsignedPermissionsTest.java | 20 +------------------ 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java b/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java index c52b4e954..9a3921b06 100644 --- a/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java +++ b/openpdf-core/src/test/java/org/openpdf/text/pdf/encryption/DecryptUnsignedPermissionsTest.java @@ -6,8 +6,6 @@ import java.io.IOException; import java.io.InputStream; -import java.lang.reflect.Field; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.openpdf.text.pdf.PdfReader; @@ -26,29 +24,13 @@ */ class DecryptUnsignedPermissionsTest { - static Field ownerPasswordUsedField; - - static boolean isOwnerPasswordUsed(PdfReader pdfReader) { - try { - return ownerPasswordUsedField.getBoolean(pdfReader); - } catch (IllegalArgumentException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } - - @BeforeAll - static void setUpBeforeClass() throws Exception { - ownerPasswordUsedField = PdfReader.class.getDeclaredField("ownerPasswordUsed"); - ownerPasswordUsedField.setAccessible(true); - } - @Test void opensWithEmptyPasswordDespiteUnsignedPermissionsValue() throws IOException { try (InputStream resource = getClass() .getResourceAsStream("/permissions/empty-user-password-unsigned-p-value.pdf")) { try (PdfReader pdfReader = new PdfReader(resource)) { assertTrue(pdfReader.isEncrypted(), "PdfReader fails to report test file to be encrypted."); - assertFalse(isOwnerPasswordUsed(pdfReader), "PdfReader fails to report limited permissions."); + assertFalse(pdfReader.isOwnerPasswordUsed(), "PdfReader fails to report limited permissions."); assertEquals(1, pdfReader.getNumberOfPages(), "PdfReader fails to report the correct number of pages"); }