Skip to content

Potential fix for code scanning alert no. 2: Server-side request forgery #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

valosnah
Copy link
Contributor

@valosnah valosnah commented Feb 7, 2025

Potential fix for https://github.com/Informasjonsforvaltning/dataset-catalog/security/code-scanning/2

To fix the SSRF vulnerability, we should ensure that the URL constructed for the HTTP request is validated against a list of authorized URLs or restricted to a particular host. In this case, since the organizationNumber is validated to be a 9-digit number, we can further ensure that the URL is restricted to the known host specified in applicationProperties.organizationCatalogHost.

We will modify the getOrganization method to validate the constructed URL against the known host before making the HTTP request.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Copilot-generert

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@valosnah valosnah requested a review from NilsOveTen February 7, 2025 13:48
return null
val url = URL("${applicationProperties.organizationCatalogHost}/organizations/$organizationNumber")
if (url.host == URL(applicationProperties.organizationCatalogHost).host) {
url.openConnection()

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

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

Copilot Autofix

AI 6 months ago

To fix the SSRF vulnerability, we need to ensure that the URL constructed from user input is strictly validated and restricted to a known safe set of URLs. One effective way to achieve this is to maintain a list of authorized organization numbers and validate the input against this list before making the request. Additionally, we should ensure that the constructed URL is limited to a particular host or more restrictive URL prefix.

  1. Maintain a list of authorized organization numbers.
  2. Validate the organizationNumber against this list before constructing the URL.
  3. Ensure the constructed URL is limited to a particular host.
Suggested changeset 1
src/main/kotlin/no/fdk/dataset_catalog/service/OrganizationService.kt

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/kotlin/no/fdk/dataset_catalog/service/OrganizationService.kt b/src/main/kotlin/no/fdk/dataset_catalog/service/OrganizationService.kt
--- a/src/main/kotlin/no/fdk/dataset_catalog/service/OrganizationService.kt
+++ b/src/main/kotlin/no/fdk/dataset_catalog/service/OrganizationService.kt
@@ -25,3 +25,3 @@
     fun getOrganization(organizationNumber: String?): Organization? {
-    if (isOrganizationNumber(organizationNumber)) {
+    if (isOrganizationNumber(organizationNumber) && isAuthorizedOrganizationNumber(organizationNumber)) {
         val url = URL("${applicationProperties.organizationCatalogHost}/organizations/$organizationNumber")
@@ -49,3 +49,3 @@
     } else {
-        logger.warn("'$organizationNumber' is not a valid organization number")
+        logger.warn("'$organizationNumber' is not a valid organization number or not authorized")
         return null
@@ -59,2 +59,6 @@
     }
+    private fun isAuthorizedOrganizationNumber(orgnr: String?): Boolean {
+        val authorizedOrganizationNumbers = listOf("123456789", "987654321") // Example list of authorized organization numbers
+        return orgnr != null && authorizedOrganizationNumbers.contains(orgnr)
+    }
 }
\ No newline at end of file
EOF
@@ -25,3 +25,3 @@
fun getOrganization(organizationNumber: String?): Organization? {
if (isOrganizationNumber(organizationNumber)) {
if (isOrganizationNumber(organizationNumber) && isAuthorizedOrganizationNumber(organizationNumber)) {
val url = URL("${applicationProperties.organizationCatalogHost}/organizations/$organizationNumber")
@@ -49,3 +49,3 @@
} else {
logger.warn("'$organizationNumber' is not a valid organization number")
logger.warn("'$organizationNumber' is not a valid organization number or not authorized")
return null
@@ -59,2 +59,6 @@
}
private fun isAuthorizedOrganizationNumber(orgnr: String?): Boolean {
val authorizedOrganizationNumbers = listOf("123456789", "987654321") // Example list of authorized organization numbers
return orgnr != null && authorizedOrganizationNumbers.contains(orgnr)
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
@valosnah
Copy link
Contributor Author

Denne er unødvendig, lukkes

@valosnah valosnah closed this Feb 12, 2025
@valosnah valosnah deleted the alert-autofix-2 branch February 12, 2025 12:24
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