Skip to content

Fix/null check proxy parsing#240

Open
krujos wants to merge 11 commits intomasterfrom
fix/null-check-proxy-parsing
Open

Fix/null check proxy parsing#240
krujos wants to merge 11 commits intomasterfrom
fix/null-check-proxy-parsing

Conversation

@krujos
Copy link
Owner

@krujos krujos commented Dec 6, 2025

Note

Upgrades the frontend stack to React 18/Bootstrap 5 with webpack 5/Babel 7 and testing-library, while switching backend logging to SLF4J and strengthening URL/proxy handling and validation.

  • Frontend:
    • Upgrade dependencies to modern stack: react@18, react-dom@18, react-bootstrap@2, bootstrap@5, redux@5, redux-thunk@3, webpack@5, Babel 7, etc.
    • Migrate components to new react-bootstrap API (PanelCard, GridContainer, progress bar changes), update HeaderBar, and use createRoot in Index.
    • Replace Enzyme with @testing-library/*; update Karma setup and launchers; include core-js polyfills.
    • Revamp webpack config (asset modules, css/less pipeline, Terser, mode/clean, source maps) and scripts.
  • Backend:
    • Replace Log4j with SLF4J; adopt HttpStatusCode and safer URI parsing in CheckedEntry.
    • Add robust proxy string parsing/validation and null checks; improve EntryChecker URL/hostname checks, proxy swapping, and timeout handling.
    • Minor util logging update in Connection.
  • Tests:
    • Rewrite React tests to @testing-library, adjust assertions, and update mocks; tweak Karma plugins/launchers.
  • Misc:
    • Remove legacy wercker.yml; add tessl.json; refresh lockfiles.

Written by Cursor Bugbot for commit c8fedea. This will update automatically on new commits. Configure here.

jkruck added 10 commits November 17, 2025 15:05
- Add safe parseProxyString() method to validate proxy format before accessing array indices
- Validate proxy string format (host:port) with proper error handling
- Add port range validation (1-65535)
- Prevent ArrayIndexOutOfBoundsException and NumberFormatException
- Centralize proxy parsing logic in reusable helper method
- Add ProxyParts inner class to safely hold parsed host and port
- Improve error logging for invalid proxy formats

Fixes unsafe array access in:
- checkHostname() method (lines 67-68)
- swapProxy() method (lines 122-123)
- Upgrade Spring Boot from 1.5.9.RELEASE to 3.3.5
- Upgrade Gradle from 8.14.3 to 9.1.0 (supports Java 25)
- Modernize build.gradle to use plugins {} block syntax
- Migrate logging from Log4j to SLF4J (Spring Boot standard)
- Update HttpStatus to HttpStatusCode for Spring Boot 3.x compatibility
- Update JSON library to latest version (20240303)
- Update Node.js plugin to modern version
- Fix null checks and unsafe array access in proxy parsing
- Add safe parseProxyString() method with validation
- Make loggers static final (best practice)
- Remove unused imports

All Java code compiles successfully with Java 25 and Gradle 9.1.0
- Updated Entry, EntryBox, EntryForm, EntryList, and HeaderBar tests to utilize React Testing Library instead of Enzyme for rendering components and querying elements.
- Enhanced test assertions to check for the presence of specific elements and their properties using screen queries.
- Improved readability and maintainability of test cases by adopting modern testing practices.
- Added @nonnull annotation to ClientHttpResponse parameters in CustomResponseErrorHandler to enforce non-nullability.
- Implemented a null check for the entry in EntryChecker, logging an error and setting canConnect to false if the entry is null.
- Refactored the getForEntity call to use the validated entry variable for improved clarity.

ResponseEntity<String> resp =
restTemplate.getForEntity(e.getEntry(), String.class);
restTemplate.getForEntity(entry, String.class);

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

Potential server-side request forgery due to a
user-provided value
.

Copilot Autofix

AI 3 months ago

The recommended way to fix SSRF vulnerabilities is to strictly restrict which URLs user input can trigger your server to contact. There are two main ways to do this:

  1. Whitelist approach: Maintain a list of allowed URLs or hosts, and only allow requests that match the whitelist.
  2. Restrict by host/subnet/scheme: Ensure that requests are only allowed to safe, intended destinations (e.g., public internet, never localhost, intranet, metadata IPs). For example, block IP ranges and DNS resolutions known to be internal or sensitive, such as private network ranges, localhost, loopback, link-local addresses, and cloud metadata endpoints.

Given the code's goal is to accept arbitrary targets, the most pragmatic fix is to restrict HTTP requests so that internal and sensitive targets are disallowed. This can be done by parsing the URL, resolving the hostname to an IP address, and rejecting requests whose host/IP falls within forbidden ranges (private, loopback, metadata endpoints). Additional scheme checks (restricting to just http/https) are also advisable.

Apply the fix in EntryChecker.checkUrl() (src/main/java/willitconnect/service/EntryChecker.java), just before making the network request:

  • Parse the URL provided by the user input.
  • Check its hostname/IP against a blacklist of forbidden addresses/ranges (e.g., localhost, internal/private IPs, link-local, 169.254.169.254 for AWS metadata, etc.).
  • Only proceed with RestTemplate.getForEntity() if the target is considered safe.
  • Set canConnect=false and log an error on blocked requests.

Add necessary imports (e.g., InetAddress, etc.) if not already present.

Suggested changeset 1
src/main/java/willitconnect/service/EntryChecker.java

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/main/java/willitconnect/service/EntryChecker.java b/src/main/java/willitconnect/service/EntryChecker.java
--- a/src/main/java/willitconnect/service/EntryChecker.java
+++ b/src/main/java/willitconnect/service/EntryChecker.java
@@ -20,6 +20,9 @@
 import java.net.InetSocketAddress;
 import java.net.Proxy;
 import java.lang.reflect.Field;
+import java.net.InetAddress;
+import java.net.URI;
+import java.net.UnknownHostException;
 import java.sql.Date;
 import java.time.Instant;
 
@@ -99,8 +102,52 @@
                 return;
             }
 
+            // SSRF protection: restrict which URLs can be accessed
+            try {
+                URI uri = new URI(entry);
+                String scheme = uri.getScheme();
+                if (scheme == null || (!scheme.equalsIgnoreCase("http") && !scheme.equalsIgnoreCase("https"))) {
+                    log.error("Blocked non-http(s) scheme in URL: " + entry);
+                    e.setCanConnect(false);
+                    return;
+                }
+                String host = uri.getHost();
+                if (host == null) {
+                    log.error("Blocked null host in URL: " + entry);
+                    e.setCanConnect(false);
+                    return;
+                }
+                InetAddress[] addresses = InetAddress.getAllByName(host);
+                for (InetAddress address : addresses) {
+                    if (address.isAnyLocalAddress()
+                            || address.isLoopbackAddress()
+                            || address.isLinkLocalAddress()
+                            || address.isSiteLocalAddress()
+                            // Block AWS/GCP/Azure metadata IPs
+                            || address.getHostAddress().equals("169.254.169.254")
+                            || address.getHostAddress().equals("127.0.0.1")
+                            || address.getHostAddress().startsWith("10.")
+                            || address.getHostAddress().startsWith("192.168.")
+                            || (address.getHostAddress().startsWith("172.")
+                                && is172Private(address.getHostAddress()))
+                        ) {
+                        log.error("Blocked SSRF attempt to forbidden IP address: " + address.getHostAddress());
+                        e.setCanConnect(false);
+                        return;
+                    }
+                }
+            } catch (UnknownHostException ex) {
+                log.error("Unknown host in URL: " + entry);
+                e.setCanConnect(false);
+                return;
+            } catch (Exception ex) {
+                log.error("Exception parsing URL for SSRF protection: " + ex.getMessage());
+                e.setCanConnect(false);
+                return;
+            }
+
             ResponseEntity<String> resp =
-                    restTemplate.getForEntity(entry, String.class);
+                restTemplate.getForEntity(entry, String.class);
 
             log.info("Status = " + resp.getStatusCode());
             e.setCanConnect(true);
@@ -115,6 +161,23 @@
         }
     }
 
+    // Helper for detecting 172.16.0.0–172.31.255.255 private addresses
+    private static boolean is172Private(String ip) {
+        // expects dotted quad string
+        // returns true if ip is in 172.16.0.0 - 172.31.255.255
+        if (!ip.startsWith("172.")) {
+            return false;
+        }
+        String[] parts = ip.split("\\.");
+        if (parts.length != 4) return false;
+        try {
+            int second = Integer.parseInt(parts[1]);
+            return second >= 16 && second <= 31;
+        } catch (NumberFormatException ex) {
+            return false;
+        }
+    }
+
     private ClientHttpRequestFactory swapProxy(CheckedEntry e) {
         ClientHttpRequestFactory oldFactory = restTemplate.getRequestFactory();
 
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on January 21

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

args = ['install', '--legacy-peer-deps', '--strict-ssl=false']
environment = [
'NODE_TLS_REJECT_UNAUTHORIZED': '0',
'GIT_TERMINAL_PROMPT': '0'
Copy link

Choose a reason for hiding this comment

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

Bug: Build disables SSL verification creating supply chain risk

The npmInstallLegacy task disables SSL certificate verification using --strict-ssl=false and NODE_TLS_REJECT_UNAUTHORIZED: '0'. This makes the build vulnerable to man-in-the-middle attacks during dependency installation, potentially allowing attackers to inject malicious packages. While this may work around certificate issues, it creates a significant supply chain security risk for all builds using this configuration.

Fix in Cursor Fix in Web

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 5 to +7
import { createStore, applyMiddleware } from 'redux';
import { Provider } from 'react-redux'
import thunk from 'redux-thunk';
import { Provider } from 'react-redux';
import { thunk } from 'redux-thunk';

Choose a reason for hiding this comment

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

P1 Badge Import redux-thunk default instead of missing named export

Redux-thunk is now imported as a named export ({ thunk }), but the package only exposes a default export; the imported value will be undefined. When applyMiddleware(thunk) runs, Redux throws at startup because the middleware is not a function, preventing the UI from rendering. Switching back to import thunk from 'redux-thunk'; avoids the runtime failure.

Useful? React with 👍 / 👎.

task webpack(type: NpmTask) {
args = ['run', 'packit']
dependsOn npmInstallLegacy
}
Copy link

Choose a reason for hiding this comment

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

Bug: Webpack task uses watch mode causing build to hang

The webpack task runs npm run packit, but the packit script in package.json includes the --watch flag (webpack --progress --watch). Since processResources depends on this task, Gradle builds will hang indefinitely because webpack never exits when running in watch mode. The previous implementation ran webpack directly without the watch flag.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant