feat: default backend none [WPB-26206] [WPB-26586] [WPB-26624]#5023
feat: default backend none [WPB-26206] [WPB-26586] [WPB-26624]#5023Garzas wants to merge 3 commits into
Conversation
Ups 🫰🟨This PR is too big. Please try to break it up into smaller PRs. |
![semgrep-code-wireapp[bot]](https://avatars.githubusercontent.com/u/16047324?s=60&v=4){kind=small}
| @Suppress("TooGenericExceptionCaught") | ||
| private suspend fun fetchSupportEmail(configUrl: String): String? = withContext(Dispatchers.IO) { | ||
| try { | ||
| val connection = URL(configUrl).openConnection() as HttpURLConnection |
There was a problem hiding this comment.
Semgrep identified a blocking 🔴 issue in your code:
Unvalidated configUrl passed directly to openConnection() allows Server-Side Request Forgery attacks targeting internal services.
More details about this
The configUrl parameter is passed directly to URL(configUrl).openConnection() without any validation of its destination. An attacker who can control the configUrl value can trick the application into making HTTP requests to arbitrary internal services or external targets.
Exploit scenario:
- An attacker provides a malicious
configUrlvalue likehttp://localhost:8080/adminorhttp://169.254.169.254/latest/meta-data(AWS metadata endpoint) - The
fetchSupportEmail()function receives this URL and immediately constructs aURLobject with it - The
openConnection()call establishes an HTTP connection to wherever the attacker specified - The application's server—which typically has network access to internal services—executes the request
- The attacker receives the response data (or discovers internal service availability), compromising internal infrastructure or exposing sensitive metadata
This works because there's no whitelist validation, URL scheme checking, or private IP range blocking before the connection is made.
To resolve this comment:
✨ Commit fix suggestion
| val connection = URL(configUrl).openConnection() as HttpURLConnection | |
| @Suppress("TooGenericExceptionCaught") | |
| private suspend fun fetchSupportEmail(configUrl: String): String? = withContext(Dispatchers.IO) { | |
| try { | |
| val uri = try { | |
| URI(configUrl) | |
| } catch (exception: Exception) { | |
| appLogger.w("Invalid backend support config URL", exception) | |
| return@withContext null | |
| } | |
| val allowedHosts = setOf( | |
| "api.example.com", | |
| "config.example.com" | |
| ) | |
| val scheme = uri.scheme?.lowercase() | |
| val host = uri.host?.lowercase() | |
| if (scheme != "https" || | |
| host.isNullOrBlank() || | |
| host !in allowedHosts || | |
| uri.userInfo != null || | |
| (uri.port != -1 && uri.port != 443) | |
| ) { | |
| appLogger.w("Rejected untrusted backend support config URL: $configUrl") | |
| return@withContext null | |
| } | |
| val connection = uri.toURL().openConnection() as HttpURLConnection | |
| connection.instanceFollowRedirects = false | |
| connection.connectTimeout = CONNECT_TIMEOUT_MILLIS | |
| connection.readTimeout = READ_TIMEOUT_MILLIS | |
| try { | |
| connection.inputStream.bufferedReader().use { reader -> | |
| json.decodeFromString(SupportConfig.serializer(), reader.readText()).supportEmail | |
| ?.trim() | |
| ?.takeIf { it.isNotBlank() } | |
| } | |
| } finally { | |
| connection.disconnect() | |
| } | |
| } catch (exception: Exception) { | |
| appLogger.w("Failed to read backend support email from config", exception) | |
| null | |
| } | |
| } |
View step-by-step instructions
-
Validate
configUrlbefore creating theURLso the app only connects to trusted destinations.
Add a small validator that parses the value withURI(configUrl)and rejects anything that is not an expectedhttpsURL. -
Restrict the host to a fixed allowlist of backend domains that your app is supposed to use.
For example, only allow hosts such asapi.example.comorconfig.example.com, and reject IP addresses,localhost, and private network names. -
Normalize and compare the parsed fields instead of doing string checks on the full URL.
Checkscheme,host, and optionallyport, for example:val uri = URI(configUrl); require(uri.scheme == "https"); require(uri.host in allowedHosts). -
Reject redirects or handle them with the same allowlist validation.
Setconnection.instanceFollowRedirects = false, or if redirects are required, read theLocationheader and validate the redirect target before following it. -
Create the connection only after validation succeeds.
MoveURL(configUrl).openConnection()behind the validation step so untrusted input never reaches the network call. -
Fail safely when the URL is invalid.
Returnnullor throw a controlled exception when validation fails, for example:if (!isTrustedConfigUrl(configUrl)) return@withContext null. -
Prefer passing a trusted config endpoint from application configuration instead of accepting an arbitrary string.
Alternatively, ifconfigUrlshould always point to one backend, replace the parameter with a constant or a value loaded from trusted app config such asURL(BuildConfig.SUPPORT_CONFIG_URL).
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by URLCONNECTION_SSRF_FD-1.
You can view more details about this finding in the Semgrep AppSec Platform.
|
1 participant
https://wearezeta.atlassian.net/browse/WPB-26206
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
Adds support for app builds that do not ship with a preconfigured default backend. When no backend is configured, the app shows a setup screen and waits for backend configuration from supported configuration sources.
Changes
default_backend_enabledconfig flag.websiteURL/support.supportEmailfrom the configuration payload without DB migration.feedback_menu_item_enabledflag to hide the generic feedback menu item.Validation