From 54a42d5a1f48da6a1aa6e81afc57ff0681ccc08e Mon Sep 17 00:00:00 2001 From: Valera V Harseko Date: Wed, 8 Jul 2026 16:04:04 +0300 Subject: [PATCH] Check ignored error status of I/O calls (CodeQL java/ignored-error-status-of-call) Nine call sites discarded the return value of a stream read/skip or of File.renameTo, so a short read, an incomplete skip, or a failed rename passed silently. Consume and act on each result: - StreamLoader.next: read the record key/value with DataInputStream.readFully instead of read(), so a short read on a buffered/socket stream can no longer hand a partially filled key or value to the import handler (latent corruption bug). - Value.ValueObjectInputStream.readFully: honour the DataInput.readFully contract - the local read() transfers exactly the requested length or throws, so a short count now raises EOFException. - IOMeter.main: loop until the requested byte count is fully skipped, since InputStream.skip may skip fewer bytes than asked. - BackupTask.rename: throw IOException when File.renameTo returns false rather than reporting success for a rename that did not happen. - WDSSO.parseToken and DerValue.init: use the byte count returned by ByteArrayInputStream.read so a truncated SPNEGO/Kerberos token or DER value fails closed instead of being compared against uninitialised trailing bytes. For every well-formed input the reads/skips/renames already completed fully, so behaviour is unchanged for existing callers; only truncated or failing I/O is now surfaced. Also add the missing 3A Systems Portions header line to StreamLoader.java, BackupTask.java, WDSSO.java and DerValue.java (Value.java and IOMeter.java already carry it). --- .../forgerock/jaspi/modules/iwa/wdsso/DerValue.java | 12 ++++++++++-- .../org/forgerock/jaspi/modules/iwa/wdsso/WDSSO.java | 11 ++++++----- .../core/src/main/java/com/persistit/BackupTask.java | 7 +++++-- .../core/src/main/java/com/persistit/IOMeter.java | 9 ++++++++- .../src/main/java/com/persistit/StreamLoader.java | 5 +++-- .../core/src/main/java/com/persistit/Value.java | 9 +++++++-- 6 files changed, 39 insertions(+), 14 deletions(-) diff --git a/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/DerValue.java b/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/DerValue.java index 5a795973c5..c866a396e2 100644 --- a/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/DerValue.java +++ b/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/DerValue.java @@ -1,4 +1,4 @@ -//@Checkstyle:ignoreFor 29 +//@Checkstyle:ignoreFor 30 /* * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. * @@ -26,12 +26,14 @@ * $Id: DerValue.java,v 1.2 2008/06/25 05:42:07 qcheng Exp $ * * Portions Copyrighted 2011-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC */ package org.forgerock.jaspi.modules.iwa.wdsso; import static java.lang.Integer.toHexString; import java.io.ByteArrayInputStream; +import java.util.Arrays; /** * A dedicated implementation of handling ASN1/DER for the @@ -88,7 +90,13 @@ private void init(ByteArrayInputStream input) { tag = (byte) input.read(); length = getLength(input); data = new byte[length]; - input.read(data, 0, length); + final int read = input.read(data, 0, length); + if (read > 0 && read < length) { + // Truncated DER value: keep only the bytes actually read so that + // callers do not treat uninitialised trailing bytes as content. + length = read; + data = Arrays.copyOf(data, read); + } } private int getLength(ByteArrayInputStream input) { diff --git a/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/WDSSO.java b/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/WDSSO.java index 13dd1e46ee..ace5e9c08e 100644 --- a/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/WDSSO.java +++ b/commons/auth-filters/authn-filter/jaspi-modules/iwa-module/src/main/java/org/forgerock/jaspi/modules/iwa/wdsso/WDSSO.java @@ -1,4 +1,4 @@ -//@Checkstyle:ignoreFor 29 +//@Checkstyle:ignoreFor 30 /* * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS HEADER. * @@ -26,6 +26,7 @@ * $Id: WindowsDesktopSSO.java,v 1.7 2009/07/28 19:40:45 beomsuk Exp $ * * Portions Copyrighted 2011-2016 ForgeRock AS. + * Portions Copyrighted 2026 3A Systems, LLC */ package org.forgerock.jaspi.modules.iwa.wdsso; @@ -285,8 +286,8 @@ private byte[] parseToken(byte[] rawToken) { // check for SPNEGO OID byte[] oidArray = new byte[SPNEGO_OID.length]; - tmpInput.read(oidArray, 0, oidArray.length); - if (Arrays.equals(oidArray, SPNEGO_OID)) { + final int oidRead = tmpInput.read(oidArray, 0, oidArray.length); + if (oidRead == oidArray.length && Arrays.equals(oidArray, SPNEGO_OID)) { tmpToken = new DerValue(tmpInput); // 0xa0 indicates an init token(NegTokenInit); 0xa1 indicates an @@ -323,8 +324,8 @@ private byte[] parseToken(byte[] rawToken) { for (; i < oidArray.length; i++) { krb5Oid[i] = oidArray[i]; } - tmpInput.read(krb5Oid, i, krb5Oid.length - i); - if (!Arrays.equals(krb5Oid, KERBEROS_V5_OID)) { + final int krb5Read = tmpInput.read(krb5Oid, i, krb5Oid.length - i); + if (krb5Read != krb5Oid.length - i || !Arrays.equals(krb5Oid, KERBEROS_V5_OID)) { LOG.debug("IWA WDSSO: Kerberos V5 OID not found in the Auth Token"); token = null; } else { diff --git a/persistit/core/src/main/java/com/persistit/BackupTask.java b/persistit/core/src/main/java/com/persistit/BackupTask.java index 2c6e7ad3be..90e307d311 100644 --- a/persistit/core/src/main/java/com/persistit/BackupTask.java +++ b/persistit/core/src/main/java/com/persistit/BackupTask.java @@ -12,6 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * Portions Copyrighted 2026 3A Systems, LLC */ package com.persistit; @@ -260,8 +261,10 @@ private void rename(final File file) throws Exception { final String candidate = k == 0 ? file.getAbsolutePath() + "~" : file.getAbsoluteFile() + "~" + k; final File newFile = new File(candidate); if (!newFile.exists()) { - file.renameTo(newFile); - return; + if (file.renameTo(newFile)) { + return; + } + throw new IOException("Unable to rename file " + file + " to " + newFile); } } throw new IOException("Unable to rename file " + file); diff --git a/persistit/core/src/main/java/com/persistit/IOMeter.java b/persistit/core/src/main/java/com/persistit/IOMeter.java index 6773ab7fdf..bbed006721 100644 --- a/persistit/core/src/main/java/com/persistit/IOMeter.java +++ b/persistit/core/src/main/java/com/persistit/IOMeter.java @@ -452,7 +452,14 @@ public static void main(final String[] args) throws Exception { } else { final IOMeter ioMeter = new IOMeter(); final DataInputStream is = new DataInputStream(new BufferedInputStream(new FileInputStream(fileName))); - is.skip(skip * DUMP_RECORD_LENGTH); + long toSkip = skip * DUMP_RECORD_LENGTH; + while (toSkip > 0) { + final long skipped = is.skip(toSkip); + if (skipped <= 0) { + break; + } + toSkip -= skipped; + } ioMeter.dump(is, count == 0 ? Integer.MAX_VALUE : count, ap.isFlag('a')); is.close(); } diff --git a/persistit/core/src/main/java/com/persistit/StreamLoader.java b/persistit/core/src/main/java/com/persistit/StreamLoader.java index f21f63da1b..a59116eebc 100644 --- a/persistit/core/src/main/java/com/persistit/StreamLoader.java +++ b/persistit/core/src/main/java/com/persistit/StreamLoader.java @@ -12,6 +12,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + * Portions Copyrighted 2026 3A Systems, LLC */ package com.persistit; @@ -136,9 +137,9 @@ public boolean next(final ImportHandler handler) throws IOException, PersistitEx final int elisionCount = _dis.readShort(); final int valueSize = _dis.readInt(); _value.ensureFit(valueSize); - _dis.read(_key.getEncodedBytes(), elisionCount, keySize - elisionCount); + _dis.readFully(_key.getEncodedBytes(), elisionCount, keySize - elisionCount); _key.setEncodedSize(keySize); - _dis.read(_value.getEncodedBytes(), 0, valueSize); + _dis.readFully(_value.getEncodedBytes(), 0, valueSize); _value.setEncodedSize(valueSize); handler.handleDataRecord(_key, _value); _dataRecordCount++; diff --git a/persistit/core/src/main/java/com/persistit/Value.java b/persistit/core/src/main/java/com/persistit/Value.java index 7e7cdb1729..91fbeb8eea 100644 --- a/persistit/core/src/main/java/com/persistit/Value.java +++ b/persistit/core/src/main/java/com/persistit/Value.java @@ -31,6 +31,7 @@ import com.persistit.util.Debug; import com.persistit.util.Util; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.NotActiveException; @@ -4891,12 +4892,16 @@ public String readUTF() { @Override public void readFully(final byte[] b) throws IOException { - read(b, 0, b.length); + readFully(b, 0, b.length); } @Override public void readFully(final byte[] b, final int offset, final int length) throws IOException { - read(b, offset, length); + // read() here either transfers exactly length bytes or throws, so a + // short count means the end of the value was reached prematurely. + if (read(b, offset, length) != length) { + throw new EOFException(); + } } @Override