From 41afd5c04619c226d3ba50cb814d59759dbda0d2 Mon Sep 17 00:00:00 2001 From: Alessandro Sangiorgi Date: Thu, 12 Mar 2026 13:01:00 -0500 Subject: [PATCH 1/3] Improve WPS execution error handling and result parsing * Add validation for `wpsReg` command response in `WpsExecutor` to catch initialization failures. * Enhance `pixiewps_wrapper.c` to handle cases where the process is killed by a signal and improve exit status logging. * Update `WpsResult` to recognize `msg=8` (M4 failure) and `msg=10` (M6 failure) strings for more accurate PIN progress tracking. * Add comprehensive unit tests in `WpsResultTest` to verify the mapping of native status outputs to Java result states (success, timeout, locked, etc.). --- lib/src/main/cpp/pixiewps/pixiewps_wrapper.c | 9 +- .../wps/lib/services/WpsExecutor.java | 5 +- .../sangiorgi/wps/lib/services/WpsResult.java | 10 +- .../wps/lib/services/WpsResultTest.java | 115 ++++++++++++++++++ 4 files changed, 133 insertions(+), 6 deletions(-) diff --git a/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c b/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c index 6a917d0..0d776b5 100644 --- a/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c +++ b/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c @@ -109,13 +109,20 @@ int pixiewps_compute(const char *pke, const char *pkr, int status; waitpid(pid, &status, 0); + if (WIFSIGNALED(status)) { + LOGE("pixiewps_compute: killed by signal %d, path=%s", WTERMSIG(status), pixiewps_exec_path); + pin_out[0] = '\0'; + return -1; + } + if (WIFEXITED(status) && WEXITSTATUS(status) == 127) { LOGE("pixiewps_compute: execl failed (exit 127), path=%s", pixiewps_exec_path); pin_out[0] = '\0'; return -1; } - LOGI("pixiewps_compute: exit=%d, output len=%d", WEXITSTATUS(status), total); + LOGI("pixiewps_compute: exit=%d, output len=%d", + WIFEXITED(status) ? WEXITSTATUS(status) : -1, total); // Parse output for "WPS pin:" line char *pin_line = strstr(buf, "WPS pin:"); diff --git a/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java b/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java index f2b0329..73ed455 100644 --- a/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java +++ b/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java @@ -122,7 +122,10 @@ private WpsResult executeNativeWps(String bssid, String pin) { SystemClock.sleep(2000); // Send WPS_REG command - wpsNative.wpsReg(bssid, pin); + String regResponse = wpsNative.wpsReg(bssid, pin); + if (regResponse == null) { + return createErrorResult(bssid, pin, "WPS_REG command failed (invalid input or shell not running)"); + } // Read WPS result with timeout sangiorgi.wps.lib.ndk.WpsResult nativeResult = diff --git a/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java b/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java index b30258e..603ef35 100644 --- a/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java +++ b/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java @@ -117,12 +117,14 @@ public boolean isLocked() { public boolean isFirstHalfCorrect() { for (CommandResult result : commandResults) { String output = result.getOutputAsString().toLowerCase(Locale.ROOT); - // M6 failure indicates first half was correct - if (output.contains("m6") || output.contains("wsc_nack after m6")) { + // M6 failure (msg=10) indicates first half was correct, second half wrong + if (output.contains("m6") || output.contains("wsc_nack after m6") + || output.contains("msg=10")) { return true; } - // M4 failure indicates first half was wrong - if (output.contains("m4") || output.contains("wsc_nack after m4")) { + // M4 failure (msg=8) indicates first half was wrong + if (output.contains("m4") || output.contains("wsc_nack after m4") + || output.contains("msg=8")) { return false; } } diff --git a/lib/src/test/java/sangiorgi/wps/lib/services/WpsResultTest.java b/lib/src/test/java/sangiorgi/wps/lib/services/WpsResultTest.java index eba3901..3219a0f 100644 --- a/lib/src/test/java/sangiorgi/wps/lib/services/WpsResultTest.java +++ b/lib/src/test/java/sangiorgi/wps/lib/services/WpsResultTest.java @@ -213,6 +213,121 @@ public void testFirstHalfCorrectDefault() { assertFalse(result.isFirstHalfCorrect()); } + // ============ Native Status Mapping (convertNativeResult output) ============ + // These tests verify that WpsResult correctly interprets the synthetic output + // produced by WpsExecutor.convertNativeResult() for each native status code. + + @Test + public void testNativeSuccessWithPassword() { + // Simulates convertNativeResult for ndk.WpsResult.Status.SUCCESS with networkKey + CommandResult cmdResult = new CommandResult( + true, + Arrays.asList("WPS-SUCCESS", "Network Key: MyPassword123"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + result.setPassword("MyPassword123"); + + assertTrue(result.isSuccess()); + assertEquals("MyPassword123", result.getPassword()); + assertFalse(result.isTimeout()); + assertFalse(result.isLocked()); + } + + @Test + public void testNativeSuccessWithoutPassword() { + // Simulates convertNativeResult for SUCCESS when networkKey is null + CommandResult cmdResult = new CommandResult( + true, + Arrays.asList("WPS-SUCCESS"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertTrue(result.isSuccess()); + assertNull(result.getPassword()); + } + + @Test + public void testNativeFourFailIsWrongPin() { + // Simulates convertNativeResult for FOUR_FAIL (msg=8 = M4 failure) + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("WPS-FAIL msg=8 config_error=18"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertTrue("FOUR_FAIL should be detected as wrong PIN", result.isWrongPin()); + assertTrue("FOUR_FAIL should be detected as PIN rejected", result.isPinRejected()); + assertFalse("FOUR_FAIL first half should NOT be correct", result.isFirstHalfCorrect()); + } + + @Test + public void testNativeThreeFailIsFirstHalfCorrect() { + // Simulates convertNativeResult for THREE_FAIL (msg=10 = M6 failure) + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("WPS-FAIL msg=10 config_error=18"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertTrue("THREE_FAIL should be detected as PIN rejected", result.isPinRejected()); + assertTrue("THREE_FAIL first half should be correct", result.isFirstHalfCorrect()); + } + + @Test + public void testNativeLockedStatus() { + // Simulates convertNativeResult for LOCKED + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("WPS-FAIL config_error=15", "setup locked"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertTrue("LOCKED should be detected", result.isLocked()); + assertEquals("WPS is locked on this router", result.getFailureReason()); + } + + @Test + public void testNativeTimeoutStatus() { + // Simulates convertNativeResult for TIMEOUT + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("WPS-TIMEOUT"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertTrue("TIMEOUT should be detected", result.isTimeout()); + } + + @Test + public void testNativeSelinuxStatus() { + // Simulates convertNativeResult for SELINUX + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("SELinux denied"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertFalse(result.isTimeout()); + } + + @Test + public void testNativeDefaultFailStatus() { + // Simulates convertNativeResult for ERROR/CRC_FAIL/unknown + CommandResult cmdResult = new CommandResult( + false, + Arrays.asList("WPS-FAIL"), + null, WpsCommand.CommandType.WPA_SUPPLICANT); + WpsResult result = new WpsResult(TEST_BSSID, TEST_PIN, Collections.singletonList(cmdResult)); + + assertFalse(result.isSuccess()); + assertTrue("Generic WPS-FAIL should be detected as PIN rejected", result.isPinRejected()); + } + // ============ Wrong PIN ============ @Test From e85796453910a8e1ec789b089fee2ebef28594db Mon Sep 17 00:00:00 2001 From: Alessandro Sangiorgi Date: Thu, 12 Mar 2026 13:31:58 -0500 Subject: [PATCH 2/3] Improve error handling for WPS registration and Pixiewps execution * Strengthen response validation in `WpsExecutor` to handle empty or "FAIL" results from `wpsReg`. * Add `waitpid` error checking and logging to `pixiewps_wrapper.c` to better handle process execution failures. --- lib/src/main/cpp/pixiewps/pixiewps_wrapper.c | 7 ++++++- .../main/java/sangiorgi/wps/lib/services/WpsExecutor.java | 6 ++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c b/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c index 0d776b5..dd489eb 100644 --- a/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c +++ b/lib/src/main/cpp/pixiewps/pixiewps_wrapper.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #define LOG_TAG "PixiewpsWrapper" @@ -107,7 +108,11 @@ int pixiewps_compute(const char *pke, const char *pkr, // Wait for child int status; - waitpid(pid, &status, 0); + if (waitpid(pid, &status, 0) < 0) { + LOGE("pixiewps_compute: waitpid failed: %s", strerror(errno)); + pin_out[0] = '\0'; + return -1; + } if (WIFSIGNALED(status)) { LOGE("pixiewps_compute: killed by signal %d, path=%s", WTERMSIG(status), pixiewps_exec_path); diff --git a/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java b/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java index 73ed455..5a9ebf3 100644 --- a/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java +++ b/lib/src/main/java/sangiorgi/wps/lib/services/WpsExecutor.java @@ -123,8 +123,10 @@ private WpsResult executeNativeWps(String bssid, String pin) { // Send WPS_REG command String regResponse = wpsNative.wpsReg(bssid, pin); - if (regResponse == null) { - return createErrorResult(bssid, pin, "WPS_REG command failed (invalid input or shell not running)"); + String trimmed = regResponse != null ? regResponse.trim() : null; + if (trimmed == null || trimmed.isEmpty() || "FAIL".equalsIgnoreCase(trimmed)) { + return createErrorResult(bssid, pin, + "WPS_REG command failed (response: " + regResponse + ")"); } // Read WPS result with timeout From e777225b7d3bd5ef5a8f3c3495554534fbbdd47b Mon Sep 17 00:00:00 2001 From: Alessandro Sangiorgi Date: Thu, 12 Mar 2026 17:47:16 -0500 Subject: [PATCH 3/3] Refine `isFirstHalfCorrect` logic to prioritize M6 results * Split the result scanning into two passes to ensure an M6 failure (indicating the first half is correct) takes precedence over an M4 failure. * This ensures correct evaluation when `WpsResult` aggregates multiple command results, such as in Pixie Dust attacks. --- lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java b/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java index 603ef35..f745544 100644 --- a/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java +++ b/lib/src/main/java/sangiorgi/wps/lib/services/WpsResult.java @@ -115,6 +115,8 @@ public boolean isLocked() { * @return true if first 4 digits are correct (M6 failure or later) */ public boolean isFirstHalfCorrect() { + // Scan all results before concluding — an M6 in any position wins over an earlier M4, + // which matters when WpsResult aggregates multiple CommandResults (e.g. Pixie Dust). for (CommandResult result : commandResults) { String output = result.getOutputAsString().toLowerCase(Locale.ROOT); // M6 failure (msg=10) indicates first half was correct, second half wrong @@ -122,6 +124,9 @@ public boolean isFirstHalfCorrect() { || output.contains("msg=10")) { return true; } + } + for (CommandResult result : commandResults) { + String output = result.getOutputAsString().toLowerCase(Locale.ROOT); // M4 failure (msg=8) indicates first half was wrong if (output.contains("m4") || output.contains("wsc_nack after m4") || output.contains("msg=8")) {