diff --git a/README.md b/README.md index 1823662..e4dc2a5 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,10 @@ The Windows Remote Management (WinRM) Java Client is a library that enables to: > will fail** during the TLS handshake unless you install the server certificate (or its issuing > CA) into a Java trust store (e.g. `-Djavax.net.ssl.trustStore=...`) or disable TLS validation > with `-Dorg.metricshub.winrm.tls.insecure=true` (**insecure — for testing only**). +> * The collections returned by `WinRMWqlExecutor.getHeaders()`/`getRows()` and +> `WqlQuery.getSelectedProperties()`/`getSubPropertiesMap()` are now **unmodifiable views** +> (and `WinRMWqlExecutor` copies the lists passed to its constructor): callers that mutated +> the returned collections must now copy them first. ## The WinRM client diff --git a/pom.xml b/pom.xml index 20a8aaf..22e0585 100644 --- a/pom.xml +++ b/pom.xml @@ -94,6 +94,14 @@ + + + com.github.spotbugs + spotbugs-annotations + 4.9.3 + provided + org.junit.platform junit-platform-launcher @@ -204,9 +212,10 @@ maven-pmd-plugin @@ -216,11 +225,13 @@ verify check + cpd-check ${maven.compiler.release} + 50 pmd.xml @@ -228,6 +239,46 @@ + + + maven-checkstyle-plugin + 3.6.0 + + + verify + + check + + + + + checkstyle.xml + + + + + + com.github.spotbugs + spotbugs-maven-plugin + 4.9.3.0 + + + verify + + check + + + + + diff --git a/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java b/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java index 9367f36..252b429 100644 --- a/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java +++ b/src/main/java/org/metricshub/winrm/ShareRemoteDirectoryConsumer.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/ShellFileCopy.java b/src/main/java/org/metricshub/winrm/ShellFileCopy.java index 6e3566f..87e135c 100644 --- a/src/main/java/org/metricshub/winrm/ShellFileCopy.java +++ b/src/main/java/org/metricshub/winrm/ShellFileCopy.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -203,7 +203,14 @@ static String copyFile( final long timeout, final long start ) throws IOException, TimeoutException, WindowsRemoteException { - final String fileName = localPath.getFileName().toString(); + // Path.getFileName() is null for a root path such as "C:\" — which has no name to stage under + final Path fileNamePath = localPath.getFileName(); + if (fileNamePath == null) { + throw new IllegalArgumentException( + String.format("Path %s has no file name and cannot be transferred to a Windows host.", localPath) + ); + } + final String fileName = fileNamePath.toString(); checkTransferableFileName(fileName); final byte[] content = Files.readAllBytes(localPath); diff --git a/src/main/java/org/metricshub/winrm/TimeoutHelper.java b/src/main/java/org/metricshub/winrm/TimeoutHelper.java index 8fe13ef..29b8e7d 100644 --- a/src/main/java/org/metricshub/winrm/TimeoutHelper.java +++ b/src/main/java/org/metricshub/winrm/TimeoutHelper.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/Utils.java b/src/main/java/org/metricshub/winrm/Utils.java index 23334fd..c9aa1ce 100644 --- a/src/main/java/org/metricshub/winrm/Utils.java +++ b/src/main/java/org/metricshub/winrm/Utils.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WinRMHttpProtocolEnum.java b/src/main/java/org/metricshub/winrm/WinRMHttpProtocolEnum.java index 3aacc55..124c0c3 100644 --- a/src/main/java/org/metricshub/winrm/WinRMHttpProtocolEnum.java +++ b/src/main/java/org/metricshub/winrm/WinRMHttpProtocolEnum.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WindowsRemoteCommandResult.java b/src/main/java/org/metricshub/winrm/WindowsRemoteCommandResult.java index c89d7aa..1d26338 100644 --- a/src/main/java/org/metricshub/winrm/WindowsRemoteCommandResult.java +++ b/src/main/java/org/metricshub/winrm/WindowsRemoteCommandResult.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WindowsRemoteProcessUtils.java b/src/main/java/org/metricshub/winrm/WindowsRemoteProcessUtils.java index c2fb68d..adf145e 100644 --- a/src/main/java/org/metricshub/winrm/WindowsRemoteProcessUtils.java +++ b/src/main/java/org/metricshub/winrm/WindowsRemoteProcessUtils.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WindowsTempShare.java b/src/main/java/org/metricshub/winrm/WindowsTempShare.java index fbcd70e..2057512 100644 --- a/src/main/java/org/metricshub/winrm/WindowsTempShare.java +++ b/src/main/java/org/metricshub/winrm/WindowsTempShare.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WmiHelper.java b/src/main/java/org/metricshub/winrm/WmiHelper.java index a912998..7fcc922 100644 --- a/src/main/java/org/metricshub/winrm/WmiHelper.java +++ b/src/main/java/org/metricshub/winrm/WmiHelper.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/WqlQuery.java b/src/main/java/org/metricshub/winrm/WqlQuery.java index bbc8bbc..7648f1b 100644 --- a/src/main/java/org/metricshub/winrm/WqlQuery.java +++ b/src/main/java/org/metricshub/winrm/WqlQuery.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -222,12 +223,27 @@ static String buildCleanWql( return cleanWql; } + /** + * Get the properties of the SELECT statement, in lower case. + * + * @return an unmodifiable view of the selected properties (empty for {@code SELECT *}) + */ public List getSelectedProperties() { - return selectedProperties; + return Collections.unmodifiableList(selectedProperties); } + /** + * Get the map of subproperties to retrieve inside each selected property, in lower case. + * + * @return an unmodifiable view of the property to subproperties map (the subproperty sets are + * unmodifiable too) + */ public Map> getSubPropertiesMap() { - return subPropertiesMap; + // Deep view: wrap each value set too, so callers cannot alter the parsed query's metadata + final Map> view = new LinkedHashMap<>(); + subPropertiesMap + .forEach((property, subProperties) -> view.put(property, Collections.unmodifiableSet(subProperties))); + return Collections.unmodifiableMap(view); } public String getCleanWql() { diff --git a/src/main/java/org/metricshub/winrm/cli/CliArguments.java b/src/main/java/org/metricshub/winrm/cli/CliArguments.java index 9ce6a4d..4335e47 100644 --- a/src/main/java/org/metricshub/winrm/cli/CliArguments.java +++ b/src/main/java/org/metricshub/winrm/cli/CliArguments.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/cli/CliUsageException.java b/src/main/java/org/metricshub/winrm/cli/CliUsageException.java index 9c45f79..61105b5 100644 --- a/src/main/java/org/metricshub/winrm/cli/CliUsageException.java +++ b/src/main/java/org/metricshub/winrm/cli/CliUsageException.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/cli/CommandLineBuilder.java b/src/main/java/org/metricshub/winrm/cli/CommandLineBuilder.java index 59ec261..5a05238 100644 --- a/src/main/java/org/metricshub/winrm/cli/CommandLineBuilder.java +++ b/src/main/java/org/metricshub/winrm/cli/CommandLineBuilder.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/cli/JsonLinesWriter.java b/src/main/java/org/metricshub/winrm/cli/JsonLinesWriter.java index ed3253d..661f550 100644 --- a/src/main/java/org/metricshub/winrm/cli/JsonLinesWriter.java +++ b/src/main/java/org/metricshub/winrm/cli/JsonLinesWriter.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/cli/WinRmCli.java b/src/main/java/org/metricshub/winrm/cli/WinRmCli.java index 534779b..41bee70 100644 --- a/src/main/java/org/metricshub/winrm/cli/WinRmCli.java +++ b/src/main/java/org/metricshub/winrm/cli/WinRmCli.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/exceptions/WinRMException.java b/src/main/java/org/metricshub/winrm/exceptions/WinRMException.java index d3b32a3..8889b19 100644 --- a/src/main/java/org/metricshub/winrm/exceptions/WinRMException.java +++ b/src/main/java/org/metricshub/winrm/exceptions/WinRMException.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/exceptions/WindowsRemoteException.java b/src/main/java/org/metricshub/winrm/exceptions/WindowsRemoteException.java index 36eedd2..e9e88f1 100644 --- a/src/main/java/org/metricshub/winrm/exceptions/WindowsRemoteException.java +++ b/src/main/java/org/metricshub/winrm/exceptions/WindowsRemoteException.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/exceptions/WqlQuerySyntaxException.java b/src/main/java/org/metricshub/winrm/exceptions/WqlQuerySyntaxException.java index eeca9d3..9598bd4 100644 --- a/src/main/java/org/metricshub/winrm/exceptions/WqlQuerySyntaxException.java +++ b/src/main/java/org/metricshub/winrm/exceptions/WqlQuerySyntaxException.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/light/CipherGen.java b/src/main/java/org/metricshub/winrm/light/CipherGen.java index 413f4fd..7e08267 100644 --- a/src/main/java/org/metricshub/winrm/light/CipherGen.java +++ b/src/main/java/org/metricshub/winrm/light/CipherGen.java @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.security.Key; import java.security.MessageDigest; import java.util.Arrays; @@ -35,6 +36,13 @@ */ public class CipherGen { + /** Shared justification for the SpotBugs EI_EXPOSE_REP/EI_EXPOSE_REP2 suppressions below */ + private static final String EXPOSE_JUSTIFICATION = "Reference NTLM crypto port kept aligned with upstream: the lazily computed hashes/responses are " + + + "cached and returned by design, and the challenge/random/targetInformation inputs are " + + "handshake-scoped byte arrays owned by the single NTLM exchange - defensive copies would churn " + + "the port for no security benefit"; + private final Random random; private final long currentTime; @@ -68,6 +76,7 @@ public class CipherGen { private byte[] ntlm2SessionResponseUserSessionKey = null; private byte[] lanManagerSessionKey = null; + @SuppressFBWarnings(value = "EI_EXPOSE_REP2", justification = EXPOSE_JUSTIFICATION) public CipherGen( final Random random, final long currentTime, @@ -104,6 +113,7 @@ private byte[] getClientChallenge2() { } /** Calculate and return random secondary key */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getSecondaryKey() { if (secondaryKey == null) { secondaryKey = makeSecondaryKey(random); @@ -120,6 +130,7 @@ private byte[] getLMHash() throws NtlmException { } /** Calculate and return the LMResponse */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getLMResponse() throws NtlmException { if (lmResponse == null) { lmResponse = lmResponse(getLMHash(), challenge); @@ -136,6 +147,7 @@ private byte[] getNTLMHash() throws NtlmException { } /** Calculate and return the NTLMResponse */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLMResponse() throws NtlmException { if (ntlmResponse == null) { ntlmResponse = lmResponse(getNTLMHash(), challenge); @@ -228,6 +240,7 @@ private static byte[] createBlob( } /** Calculate the NTLMv2Response */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLMv2Response() throws NtlmException { if (ntlmv2Response == null) { ntlmv2Response = lmv2Response(getNTLMv2Hash(), challenge, getNTLMv2Blob()); @@ -236,6 +249,7 @@ public byte[] getNTLMv2Response() throws NtlmException { } /** Calculate the LMv2Response */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getLMv2Response() throws NtlmException { if (lmv2Response == null) { lmv2Response = lmv2Response(getLMv2Hash(), challenge, getClientChallenge()); @@ -244,6 +258,7 @@ public byte[] getLMv2Response() throws NtlmException { } /** Get NTLM2SessionResponse */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLM2SessionResponse() throws NtlmException { if (ntlm2SessionResponse == null) { ntlm2SessionResponse = ntlm2SessionResponse(getNTLMHash(), challenge, getClientChallenge()); @@ -395,6 +410,7 @@ private static void oddParity(final byte[] bytes) { } /** Calculate and return LM2 session response */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getLM2SessionResponse() { if (lm2SessionResponse == null) { final byte[] clntChallenge = getClientChallenge(); @@ -406,6 +422,7 @@ public byte[] getLM2SessionResponse() { } /** Get LMUserSessionKey */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getLMUserSessionKey() throws NtlmException { if (lmUserSessionKey == null) { lmUserSessionKey = new byte[16]; @@ -416,6 +433,7 @@ public byte[] getLMUserSessionKey() throws NtlmException { } /** Get NTLMUserSessionKey */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLMUserSessionKey() throws NtlmException { if (ntlmUserSessionKey == null) { final MD4 md4 = new MD4(); @@ -426,6 +444,7 @@ public byte[] getNTLMUserSessionKey() throws NtlmException { } /** GetNTLMv2UserSessionKey */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLMv2UserSessionKey() throws NtlmException { if (ntlmv2UserSessionKey == null) { final byte[] ntlmv2hash = getNTLMv2Hash(); @@ -437,6 +456,7 @@ public byte[] getNTLMv2UserSessionKey() throws NtlmException { } /** Get NTLM2SessionResponseUserSessionKey */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getNTLM2SessionResponseUserSessionKey() throws NtlmException { if (ntlm2SessionResponseUserSessionKey == null) { final byte[] ntlm2SessionResponseNonce = getLM2SessionResponse(); @@ -449,6 +469,7 @@ public byte[] getNTLM2SessionResponseUserSessionKey() throws NtlmException { } /** Get LAN Manager session key */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = EXPOSE_JUSTIFICATION) public byte[] getLanManagerSessionKey() throws NtlmException { if (lanManagerSessionKey == null) { try { diff --git a/src/main/java/org/metricshub/winrm/light/MD4.java b/src/main/java/org/metricshub/winrm/light/MD4.java index 44e8e42..45afbf4 100644 --- a/src/main/java/org/metricshub/winrm/light/MD4.java +++ b/src/main/java/org/metricshub/winrm/light/MD4.java @@ -63,7 +63,6 @@ void update(final byte[] input) { final int transferAmt = input.length - inputIndex; System.arraycopy(input, inputIndex, dataBuffer, curBufferPos, transferAmt); count += transferAmt; - curBufferPos += transferAmt; } } diff --git a/src/main/java/org/metricshub/winrm/light/NTLMMessage.java b/src/main/java/org/metricshub/winrm/light/NTLMMessage.java index b5cdc1f..db860b8 100644 --- a/src/main/java/org/metricshub/winrm/light/NTLMMessage.java +++ b/src/main/java/org/metricshub/winrm/light/NTLMMessage.java @@ -20,6 +20,8 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * NTLM message generation, base class * Code from io.cloudsoft.winrm4j.client.ntlm.forks.httpclient.NTLMEngineImpl @@ -50,6 +52,10 @@ class NTLMMessage { NTLMMessage() {} /** Constructor to use when message bytes are known */ + @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Parse-in-constructor is the upstream NTLM engine design: rejecting a malformed " + + + "message must abort construction. The class is package-private, so no attacker subclass can " + + "mount a finalizer attack on the partially-built instance") NTLMMessage(final byte[] message, final int expectedType) throws NtlmException { messageContents = message; // Look for NTLM message diff --git a/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java b/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java index 5754027..c79ab83 100644 --- a/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java +++ b/src/main/java/org/metricshub/winrm/light/NtlmCrypto.java @@ -44,10 +44,12 @@ static byte[] encryptAndSign(final WinRMSession session, final byte[] messageBod try (ByteArrayOutputStream out = new ByteArrayOutputStream()) { out.write(BOUNDARY_CR.getBytes(StandardCharsets.US_ASCII)); out.write("\tContent-Type: application/HTTP-SPNEGO-session-encrypted\r\n".getBytes(StandardCharsets.US_ASCII)); + // Plain concatenation (not String.format) so the protocol-mandated CRLF cannot be mistaken + // for a platform line separator out.write( - String - .format("\tOriginalContent: type=application/soap+xml;charset=UTF-8;Length=%d\r\n", messageBody.length) - .getBytes(StandardCharsets.US_ASCII) + ("\tOriginalContent: type=application/soap+xml;charset=UTF-8;Length=" + messageBody.length + "\r\n").getBytes( + StandardCharsets.US_ASCII + ) ); out.write(BOUNDARY_CR.getBytes(StandardCharsets.US_ASCII)); out.write("\tContent-Type: application/octet-stream\r\n".getBytes(StandardCharsets.US_ASCII)); diff --git a/src/main/java/org/metricshub/winrm/light/Type2Message.java b/src/main/java/org/metricshub/winrm/light/Type2Message.java index 5f069c3..80419be 100644 --- a/src/main/java/org/metricshub/winrm/light/Type2Message.java +++ b/src/main/java/org/metricshub/winrm/light/Type2Message.java @@ -20,6 +20,8 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + /** * Type 2 message class * Code from io.cloudsoft.winrm4j.client.ntlm.forks.httpclient.NTLMEngineImpl @@ -33,10 +35,18 @@ class Type2Message extends NTLMMessage { private byte[] targetInfo; private final int flags; + @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Parse-in-constructor is the upstream NTLM engine design: rejecting a malformed " + + + "server challenge must abort construction. The class is package-private, so no attacker subclass " + + "can mount a finalizer attack on the partially-built instance") Type2Message(final String messageBody) throws NtlmException { this(java.util.Base64.getDecoder().decode(messageBody.getBytes(NTLMEngineUtils.DEFAULT_CHARSET))); } + @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Parse-in-constructor is the upstream NTLM engine design: rejecting a malformed " + + + "server challenge must abort construction. The class is package-private, so no attacker subclass " + + "can mount a finalizer attack on the partially-built instance") private Type2Message(final byte[] message) throws NtlmException { super(message, 2); // Type 2 message is laid out as follows: diff --git a/src/main/java/org/metricshub/winrm/light/Type3Message.java b/src/main/java/org/metricshub/winrm/light/Type3Message.java index d805a48..d313558 100644 --- a/src/main/java/org/metricshub/winrm/light/Type3Message.java +++ b/src/main/java/org/metricshub/winrm/light/Type3Message.java @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.nio.charset.Charset; import java.util.Locale; import java.util.Random; @@ -61,6 +62,10 @@ public class Type3Message extends NTLMMessage { /** * More primitive constructor: don't include cert or previous messages. */ + @SuppressFBWarnings(value = "CT_CONSTRUCTOR_THROW", justification = "Computing the NTLM responses in the constructor is the upstream NTLM engine " + + + "design: a failed crypto step must abort construction. The only constructor is package-private, " + + "so no attacker subclass can mount a finalizer attack on the partially-built instance") Type3Message( final String domain, final String host, diff --git a/src/main/java/org/metricshub/winrm/light/WinRMSession.java b/src/main/java/org/metricshub/winrm/light/WinRMSession.java index ab165a7..b1a2700 100644 --- a/src/main/java/org/metricshub/winrm/light/WinRMSession.java +++ b/src/main/java/org/metricshub/winrm/light/WinRMSession.java @@ -49,12 +49,16 @@ final class WinRMSession { private final String username; private final String password; - private long negotiateFlags; - private byte[] clientSigningKey; - private byte[] serverSigningKey; - private Cipher encryptor; - private Cipher decryptor; - private boolean authenticated; + // volatile: the session outlives individual operations, and each operation runs on a fresh + // worker thread (Utils.execute spawns one per call), so state written during the handshake on + // one thread must be visible to the thread running the next operation. Accesses are never + // concurrent (operations on a connection are sequential); only visibility is needed. + private volatile long negotiateFlags; + private volatile byte[] clientSigningKey; + private volatile byte[] serverSigningKey; + private volatile Cipher encryptor; + private volatile Cipher decryptor; + private volatile boolean authenticated; private final AtomicLong sequenceOutgoing = new AtomicLong(-1); private final AtomicLong sequenceIncoming = new AtomicLong(-1); diff --git a/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java b/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java index 7c73bd1..213bd04 100644 --- a/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java +++ b/src/main/java/org/metricshub/winrm/service/WinRMEndpoint.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,6 +20,7 @@ * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ */ +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.net.MalformedURLException; import java.net.URL; import java.util.Arrays; @@ -53,6 +54,12 @@ public class WinRMEndpoint { * @param password The password (mandatory) * @param namespace The namespace default value: {@value WmiHelper#DEFAULT_NAMESPACE} */ + @SuppressFBWarnings(value = { + "EI_EXPOSE_REP2", + "CT_CONSTRUCTOR_THROW" }, justification = "The password char[] is deliberately shared, not copied, so the caller can wipe " + + + "the single authoritative copy of the secret; and the constructor intentionally validates its " + + "arguments — no partially-initialized instance escapes since nothing publishes 'this'") public WinRMEndpoint( final WinRMHttpProtocolEnum protocol, final String hostname, @@ -114,6 +121,9 @@ public String getUsername() { } /** Get the password */ + @SuppressFBWarnings(value = "EI_EXPOSE_REP", justification = "The password char[] is deliberately handed out by reference so there is a " + + + "single wipeable copy of the secret; cloning it here would scatter copies of the password in memory") public char[] getPassword() { return password; } diff --git a/src/main/java/org/metricshub/winrm/service/client/auth/AuthenticationEnum.java b/src/main/java/org/metricshub/winrm/service/client/auth/AuthenticationEnum.java index 51b63e3..0cc0684 100644 --- a/src/main/java/org/metricshub/winrm/service/client/auth/AuthenticationEnum.java +++ b/src/main/java/org/metricshub/winrm/service/client/auth/AuthenticationEnum.java @@ -4,7 +4,7 @@ * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ * WinRM Java Client * ჻჻჻჻჻჻ - * Copyright 2023 - 2026 MetricsHub + * Copyright (C) 2023 - 2026 MetricsHub * ჻჻჻჻჻჻ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java b/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java index 6031032..fb8e83e 100644 --- a/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java +++ b/src/main/java/org/metricshub/winrm/wql/WinRMWqlExecutor.java @@ -21,6 +21,8 @@ */ import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.TimeoutException; @@ -46,13 +48,19 @@ public class WinRMWqlExecutor { * The WinRMWqlExecutor constructor * * @param executionTime The execution time in milliseconds - * @param headers The headers list - * @param rows The value rows list + * @param headers The headers list (copied defensively) + * @param rows The value rows list (copied defensively) */ public WinRMWqlExecutor(final long executionTime, final List headers, final List> rows) { this.executionTime = executionTime; - this.headers = headers; - this.rows = rows; + this.headers = headers != null ? new ArrayList<>(headers) : null; + // Deep copy: each row is copied too, so a caller retaining a row list cannot mutate this result + this.rows = rows != null + ? rows + .stream() + .map(row -> row != null ? Collections.unmodifiableList(new ArrayList<>(row)) : null) + .collect(Collectors.toList()) + : null; } /** @@ -67,19 +75,19 @@ public long getExecutionTime() { /** * Get the headers of the query. * - * @return + * @return an unmodifiable view of the headers list */ public List getHeaders() { - return headers; + return headers != null ? Collections.unmodifiableList(headers) : null; } /** * Get the result rows of the query. * - * @return + * @return an unmodifiable view of the result rows */ public List> getRows() { - return rows; + return rows != null ? Collections.unmodifiableList(rows) : null; } /** diff --git a/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java b/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java index 515a29b..c73a085 100644 --- a/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java +++ b/src/test/java/org/metricshub/winrm/ShellFileCopyTest.java @@ -348,6 +348,19 @@ void returnsCommandUnchangedWithoutFiles() throws Exception { assertTrue(executor.getExecutedCommands().isEmpty()); } + @Test + void rejectsPathWithoutFileName() { + // No handler registered: any remote interaction would fail the test + final ScriptedWindowsRemoteExecutor executor = new ScriptedWindowsRemoteExecutor(); + + // A root path has no file name component (Path.getFileName() is null) + assertThrows( + IllegalArgumentException.class, + () -> ShellFileCopy.copyFile(executor, Path.of("C:\\"), "C:\\Windows\\Temp", TIMEOUT, 0L) + ); + assertTrue(executor.getExecutedCommands().isEmpty()); + } + @Test void rejectsFileNamesUnsafeForTheCommandShell() { assertThrows(IllegalArgumentException.class, () -> ShellFileCopy.checkTransferableFileName("we%ird.txt")); diff --git a/src/test/java/org/metricshub/winrm/WqlQueryTest.java b/src/test/java/org/metricshub/winrm/WqlQueryTest.java new file mode 100644 index 0000000..02520ad --- /dev/null +++ b/src/test/java/org/metricshub/winrm/WqlQueryTest.java @@ -0,0 +1,51 @@ +package org.metricshub.winrm; + +/*- + * ╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲ + * WinRM Java Client + * ჻჻჻჻჻჻ + * Copyright 2023 - 2026 MetricsHub + * ჻჻჻჻჻჻ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * 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. + * ╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱╲╱ + */ + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.junit.jupiter.api.Test; + +class WqlQueryTest { + + @Test + void gettersReturnUnmodifiableViews() throws Exception { + final WqlQuery query = WqlQuery.newInstance("SELECT PropA, PropB.Sub1, PropB.Sub2 FROM Win32_Class"); + + final List selectedProperties = query.getSelectedProperties(); + assertEquals(List.of("propa", "propb.sub1", "propb.sub2"), selectedProperties); + assertThrows(UnsupportedOperationException.class, () -> selectedProperties.set(0, "other")); + + // The map view is unmodifiable down to each subproperty set + final Map> subPropertiesMap = query.getSubPropertiesMap(); + assertEquals(Set.of("sub1", "sub2"), subPropertiesMap.get("propb")); + assertThrows(UnsupportedOperationException.class, () -> subPropertiesMap.remove("propa")); + assertThrows(UnsupportedOperationException.class, () -> subPropertiesMap.get("propb").clear()); + + // The failed mutation attempts did not alter the parsed query + assertEquals(List.of("propa", "propb.sub1", "propb.sub2"), query.getSelectedProperties()); + assertEquals(Set.of("sub1", "sub2"), query.getSubPropertiesMap().get("propb")); + } +} diff --git a/src/test/java/org/metricshub/winrm/wql/WinRMWqlExecutorTest.java b/src/test/java/org/metricshub/winrm/wql/WinRMWqlExecutorTest.java index 7aa6afd..ce8393f 100644 --- a/src/test/java/org/metricshub/winrm/wql/WinRMWqlExecutorTest.java +++ b/src/test/java/org/metricshub/winrm/wql/WinRMWqlExecutorTest.java @@ -30,6 +30,28 @@ class WinRMWqlExecutorTest { + @Test + void resultCollectionsAreDefensivelyCopiedAndUnmodifiable() { + final List headers = new ArrayList<>(asList("Name", "Path")); + final List> rows = new ArrayList<>(); + rows.add(new ArrayList<>(asList("C$", "C:\\"))); + + final WinRMWqlExecutor result = new WinRMWqlExecutor(42L, headers, rows); + + // Mutating the source collections (including a retained inner row) after construction + // must not affect the result + headers.add("Extra"); + rows.get(0).set(0, "hacked"); + rows.add(new ArrayList<>()); + assertEquals(asList("Name", "Path"), result.getHeaders()); + assertEquals(singletonList(asList("C$", "C:\\")), result.getRows()); + + // The returned collections are unmodifiable, down to each row + assertThrows(UnsupportedOperationException.class, () -> result.getHeaders().add("x")); + assertThrows(UnsupportedOperationException.class, () -> result.getRows().add(asList("x"))); + assertThrows(UnsupportedOperationException.class, () -> result.getRows().get(0).set(0, "x")); + } + @Test void testExecute() throws Exception { final String wqlQuery = "Select Name,Path from Win32_Share";