From 574e8c7bdaf4d91fb45c91d4076c514cedaa1f7c Mon Sep 17 00:00:00 2001 From: reecexlm <98427531+reecexlm@users.noreply.github.com> Date: Thu, 10 Aug 2023 10:08:05 -0700 Subject: [PATCH] [ANCHOR-403] Prevent SSRF through SEP-1 TOML redirect (#1043) * ANCHOR-403-reece-SSRF-metadata * use assertThrows * additional cleanup * remove custom exception * make sure that the log event is actually captured by adding a verification step before the assertions * temporary setting toml to url to test * just testing. ignore * try other file * patch toml parser as well * replace comment * revert * lint * debugF * revert config changes * revert defaults --- .../java/org/stellar/anchor/util/NetUtil.java | 9 +++- .../org/stellar/anchor/util/Sep1Helper.java | 8 +++- .../org/stellar/anchor/util/NetUtilTest.kt | 25 +++++++--- .../controller/sep/Sep1Controller.java | 3 +- .../org/stellar/anchor/Sep1ServiceTest.kt | 47 ++++++++++++++++++- .../anchor/platform/LogAppenderTest.kt | 4 ++ .../anchor/platform/TestProfileRunner.kt | 5 +- 7 files changed, 88 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/stellar/anchor/util/NetUtil.java b/core/src/main/java/org/stellar/anchor/util/NetUtil.java index b8047fea6e..25de3be9cf 100644 --- a/core/src/main/java/org/stellar/anchor/util/NetUtil.java +++ b/core/src/main/java/org/stellar/anchor/util/NetUtil.java @@ -17,7 +17,14 @@ public static String fetch(String url) throws IOException { Request request = OkHttpUtil.buildGetRequest(url); Response response = getCall(request).execute(); - if (response.body() == null) return ""; + // Check if response was successful (status code 200) + if (!response.isSuccessful()) { + throw new IOException(String.format("Unable to fetch data from %s", url)); + } + + if (response.body() == null) { + throw new IOException("Response body is empty."); + } return Objects.requireNonNull(response.body()).string(); } diff --git a/core/src/main/java/org/stellar/anchor/util/Sep1Helper.java b/core/src/main/java/org/stellar/anchor/util/Sep1Helper.java index 517a0864f3..f372530ea4 100644 --- a/core/src/main/java/org/stellar/anchor/util/Sep1Helper.java +++ b/core/src/main/java/org/stellar/anchor/util/Sep1Helper.java @@ -10,7 +10,13 @@ public static TomlContent readToml(String url) throws IOException { } public static TomlContent parse(String tomlString) { - return new TomlContent(tomlString); + try { + return new TomlContent(tomlString); + } catch (Exception e) { + // obfuscate exception message to prevent metadata leaks + throw new RuntimeException( + "Failed to parse TOML content"); // or return null or a default TomlContent instance + } } public static class TomlContent { diff --git a/core/src/test/kotlin/org/stellar/anchor/util/NetUtilTest.kt b/core/src/test/kotlin/org/stellar/anchor/util/NetUtilTest.kt index 0a39404fc6..db6775b1f5 100644 --- a/core/src/test/kotlin/org/stellar/anchor/util/NetUtilTest.kt +++ b/core/src/test/kotlin/org/stellar/anchor/util/NetUtilTest.kt @@ -4,11 +4,13 @@ package org.stellar.anchor.util import io.mockk.* import io.mockk.impl.annotations.MockK +import java.io.IOException import java.net.MalformedURLException import okhttp3.Response import okhttp3.ResponseBody import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.params.ParameterizedTest @@ -36,15 +38,17 @@ internal class NetUtilTest { } @Test - fun `test fetch()`() { + fun `test fetch successful response`() { mockkStatic(NetUtil::class) every { NetUtil.getCall(any()) } returns mockCall every { mockCall.execute() } returns mockResponse + every { mockResponse.isSuccessful } returns true every { mockResponse.body } returns mockResponseBody every { mockResponseBody.string() } returns "result" val result = NetUtil.fetch("http://hello") - assert(result.equals("result")) + assertEquals("result", result) + verify { NetUtil.getCall(any()) mockCall.execute() @@ -52,15 +56,24 @@ internal class NetUtilTest { } @Test - fun `test fetch() throws exception`() { + fun `test fetch unsuccessful response`() { + mockkStatic(NetUtil::class) + every { NetUtil.getCall(any()) } returns mockCall + every { mockCall.execute() } returns mockResponse + every { mockResponse.isSuccessful } returns false + + assertThrows(IOException::class.java) { NetUtil.fetch("http://hello") } + } + + @Test + fun `test fetch null response body`() { mockkStatic(NetUtil::class) every { NetUtil.getCall(any()) } returns mockCall every { mockCall.execute() } returns mockResponse + every { mockResponse.isSuccessful } returns true every { mockResponse.body } returns null - every { mockResponseBody.string() } returns "result" - val result = NetUtil.fetch("http://hello") - assert(result.equals("")) + assertThrows(IOException::class.java) { NetUtil.fetch("http://hello") } } @Test diff --git a/platform/src/main/java/org/stellar/anchor/platform/controller/sep/Sep1Controller.java b/platform/src/main/java/org/stellar/anchor/platform/controller/sep/Sep1Controller.java index 7351d2b430..fb20aa5695 100644 --- a/platform/src/main/java/org/stellar/anchor/platform/controller/sep/Sep1Controller.java +++ b/platform/src/main/java/org/stellar/anchor/platform/controller/sep/Sep1Controller.java @@ -1,5 +1,6 @@ package org.stellar.anchor.platform.controller.sep; +import java.io.IOException; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; @@ -40,7 +41,7 @@ public RedirectView landingPage() throws SepNotFoundException { @RequestMapping( value = "/.well-known/stellar.toml", method = {RequestMethod.GET, RequestMethod.OPTIONS}) - public ResponseEntity getToml() throws SepNotFoundException { + public ResponseEntity getToml() throws IOException, SepNotFoundException { if (!sep1Config.isEnabled()) { throw new SepNotFoundException("Not Found"); } diff --git a/platform/src/test/kotlin/org/stellar/anchor/Sep1ServiceTest.kt b/platform/src/test/kotlin/org/stellar/anchor/Sep1ServiceTest.kt index 279a8cfc33..6cce8c63c4 100644 --- a/platform/src/test/kotlin/org/stellar/anchor/Sep1ServiceTest.kt +++ b/platform/src/test/kotlin/org/stellar/anchor/Sep1ServiceTest.kt @@ -1,10 +1,12 @@ package org.stellar.anchor +import java.io.IOException import java.nio.file.Files import java.nio.file.Path import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Test import org.stellar.anchor.config.Sep1Config.TomlType.* import org.stellar.anchor.platform.config.PropertySep1Config @@ -13,6 +15,7 @@ import org.stellar.anchor.platform.config.Sep1ConfigTest import org.stellar.anchor.sep1.Sep1Service class Sep1ServiceTest { + lateinit var sep1: Sep1Service val stellarToml = """ @@ -47,7 +50,6 @@ class Sep1ServiceTest { @Test fun `test Sep1Service reading toml from inline string`() { - val config = PropertySep1Config(true, TomlConfig(STRING, stellarToml)) sep1 = Sep1Service(config) assertEquals(sep1.stellarToml, stellarToml) @@ -66,9 +68,50 @@ class Sep1ServiceTest { mockServer.start() val mockAnchorUrl = mockServer.url("").toString() mockServer.enqueue(MockResponse().setBody(stellarToml)) - val config = PropertySep1Config(true, TomlConfig(URL, mockAnchorUrl)) sep1 = Sep1Service(config) assertEquals(sep1.stellarToml, stellarToml) } + + @Test + fun `getStellarToml throws exception when redirect is encountered`() { + val mockServer = MockWebServer() + mockServer.start() + val mockAnchorUrl = mockServer.url("").toString() + + // Enqueue a response with a 302 status and a Location header to simulate a redirect. + mockServer.enqueue( + MockResponse() + .setResponseCode(302) + .setHeader("Location", mockServer.url("/new_location").toString()) + ) + + // Enqueue a response at the redirect location that provides a server error. + mockServer.enqueue(MockResponse().setResponseCode(500)) + + // Enqueue an empty response to prevent a timeout. + mockServer.enqueue(MockResponse()) + + val config = PropertySep1Config(true, TomlConfig(URL, mockAnchorUrl)) + + val exception = assertThrows(IOException::class.java) { Sep1Service(config) } + assertEquals("Unable to fetch data from $mockAnchorUrl", exception.message) + mockServer.shutdown() + } + + @Test + fun `getStellarToml throws exception when redirected location results in error`() { + val mockServer = MockWebServer() + mockServer.start() + val mockAnchorUrl = mockServer.url("/new_location").toString() + + // Enqueue a response that provides a server error. + mockServer.enqueue(MockResponse().setResponseCode(500)) + + val config = PropertySep1Config(true, TomlConfig(URL, mockAnchorUrl)) + + val exception = assertThrows(IOException::class.java) { sep1 = Sep1Service(config) } + assertEquals("Unable to fetch data from $mockAnchorUrl", exception.message) + mockServer.shutdown() + } } diff --git a/platform/src/test/kotlin/org/stellar/anchor/platform/LogAppenderTest.kt b/platform/src/test/kotlin/org/stellar/anchor/platform/LogAppenderTest.kt index 824fe940e5..c07703a8ea 100644 --- a/platform/src/test/kotlin/org/stellar/anchor/platform/LogAppenderTest.kt +++ b/platform/src/test/kotlin/org/stellar/anchor/platform/LogAppenderTest.kt @@ -68,6 +68,10 @@ class LogAppenderTest { "debug" -> Log.debug(wantMessage) "trace" -> Log.trace(wantMessage) } + + // Verify that the append method was called, which ensures that the event was captured + verify { appender.append(any()) } + assertEquals(LogAppenderTest::class.qualifiedName, capturedLogEvent.captured.loggerName) assertEquals(wantLevelName, capturedLogEvent.captured.level.toString()) assertEquals(wantMessage, capturedLogEvent.captured.message.toString()) diff --git a/service-runner/src/main/kotlin/org/stellar/anchor/platform/TestProfileRunner.kt b/service-runner/src/main/kotlin/org/stellar/anchor/platform/TestProfileRunner.kt index b9c37208c6..a209a5d077 100644 --- a/service-runner/src/main/kotlin/org/stellar/anchor/platform/TestProfileRunner.kt +++ b/service-runner/src/main/kotlin/org/stellar/anchor/platform/TestProfileRunner.kt @@ -73,8 +73,9 @@ class TestProfileExecutor(val config: TestConfig) { val envMap = config.env envMap["assets.value"] = getResourceFile(envMap["assets.value"]!!).absolutePath - envMap["sep1.toml.value"] = getResourceFile(envMap["sep1.toml.value"]!!).absolutePath - + if (envMap["sep1.toml.type"] != "url") { + envMap["sep1.toml.value"] = getResourceFile(envMap["sep1.toml.value"]!!).absolutePath + } // Start servers val jobs = mutableListOf() val scope = CoroutineScope(Dispatchers.Default)