Skip to content

fix(security): block SSRF via send-test-email SMTP host validation (GHSA-vvxf-f8q9-86gh) #41666

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.

Sign up for GitHub

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

Open
subrata71 wants to merge 5 commits into
base: release
Choose a base branch
from
+200 −6
Open

fix(security): block SSRF via send-test-email SMTP host validation (GHSA-vvxf-f8q9-86gh) #41666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ce761a5
fix(security): block SSRF via send-test-email SMTP host validation
subrata71 Mar 27, 2026
524cc1b
fix(security): address review - TOCTOU, IPv6 ULA, Cypress test failure
subrata71 Mar 28, 2026
aba623c
Merge remote-tracking branch 'origin/release' into fix/ssrf-send-test…
subrata71 Mar 30, 2026
08f7868
fix(security): add SMTP port allowlist as defense-in-depth
subrata71 Mar 30, 2026
ead9025
fix(security): enable implicit TLS for SMTPS on port 465
subrata71 Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions app/server/appsmith-interfaces/src/main/java/com/appsmith/util/WebClientUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Optional;
import java.util.Set;

@Slf4j
Expand Down Expand Up @@ -198,6 +199,57 @@ protected AddressResolver<InetSocketAddress> newResolver(EventExecutor executor)
}
}

/**
* Resolves a hostname and validates that none of its addresses are disallowed for
* outbound connections from non-HTTP paths (e.g. SMTP via JavaMail). Checks against
* the cloud-metadata denylist, loopback, link-local, any-local, multicast, and IPv6
* Unique Local Addresses (fc00::/7). Returns the first validated resolved address so
* callers can connect to it directly, preventing DNS-rebinding TOCTOU bypasses.
*
* <p>RFC 1918 site-local ranges (10/8, 172.16/12, 192.168/16) are intentionally
* allowed because legitimate SMTP servers frequently reside on private networks.
*
* @return the resolved {@link InetAddress} if the host is allowed, or empty if blocked
*/
public static Optional<InetAddress> resolveIfAllowed(String host) {
if (!StringUtils.hasText(host)) {
return Optional.empty();
}

final String canonicalHost = normalizeHostForComparisonQuietly(host);

if (DISALLOWED_HOSTS.contains(canonicalHost)) {
return Optional.empty();
}

final InetAddress[] resolved;
try {
resolved = InetAddress.getAllByName(host);
} catch (UnknownHostException e) {
return Optional.empty();
}

for (InetAddress addr : resolved) {
if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(addr.getHostAddress()))) {
return Optional.empty();
}
if (addr instanceof Inet6Address) {
byte firstByte = addr.getAddress()[0];
if ((firstByte & (byte) 0xFE) == (byte) 0xFC) {
return Optional.empty();
}
}
if (addr.isLoopbackAddress()
|| addr.isLinkLocalAddress()
|| addr.isAnyLocalAddress()
|| addr.isMulticastAddress()) {
return Optional.empty();
}
}

return Optional.of(resolved[0]);
}

public static boolean isDisallowedAndFail(String host, Promise<?> promise) {
if (DISALLOWED_HOSTS.contains(normalizeHostForComparisonQuietly(host))) {
log.warn("Host {} is disallowed. Failing the request.", host);
Expand Down
51 changes: 51 additions & 0 deletions app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.appsmith.util;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.http.HttpMethod;
Expand All @@ -8,8 +9,10 @@
import reactor.test.StepVerifier;

import java.lang.reflect.Method;
import java.net.InetAddress;
import java.net.URI;
import java.net.UnknownHostException;
import java.util.Optional;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
Expand Down Expand Up @@ -43,6 +46,54 @@ public void testIsDisallowedAndFailNormalizesMetadataHostnames(String host) {
assertTrue(WebClientUtils.isDisallowedAndFail(host, null));
}

@ParameterizedTest
@ValueSource(
strings = {
"127.0.0.1",
"169.254.169.254",
"169.254.10.10",
"100.100.100.200",
"168.63.129.16",
"0.0.0.0",
})
public void resolveIfAllowed_blocksLoopbackMetadataAndSpecialHosts(String host) {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed(host);
assertTrue(result.isEmpty(), "Expected host " + host + " to be blocked");
}

@Test
public void resolveIfAllowed_blocksNullAndEmpty() {
assertTrue(WebClientUtils.resolveIfAllowed(null).isEmpty());
assertTrue(WebClientUtils.resolveIfAllowed("").isEmpty());
assertTrue(WebClientUtils.resolveIfAllowed(" ").isEmpty());
}

@Test
public void resolveIfAllowed_blocksLocalhostHostname() {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("localhost");
assertTrue(result.isEmpty(), "Expected 'localhost' to be blocked");
}

@ParameterizedTest
@ValueSource(strings = {"smtp.gmail.com", "email-smtp.us-east-1.amazonaws.com", "smtp.sendgrid.net"})
public void resolveIfAllowed_allowsLegitimateSmtpHosts(String host) {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed(host);
assertTrue(result.isPresent(), "Expected host " + host + " to be allowed");
}

@Test
public void resolveIfAllowed_blocksUnresolvableHost() {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("definitely-not-a-real-host-xyz123.invalid");
assertTrue(result.isEmpty(), "Expected unresolvable host to be blocked");
}

@Test
public void resolveIfAllowed_returnsResolvedAddress() {
Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com");
assertTrue(result.isPresent());
assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+"));
}
Comment on lines +90 to +95
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

IPv4-only regex may fail for IPv6-capable resolvers.

The regex \\d+\\.\\d+\\.\\d+\\.\\d+ assumes IPv4. If smtp.gmail.com resolves to an IPv6 address (Google supports both), this assertion fails.

🛡️ Proposed fix to handle both IPv4 and IPv6 
     `@Test`
     public void resolveIfAllowed_returnsResolvedAddress() {
         Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com");
         assertTrue(result.isPresent());
-        assertTrue(result.get().getHostAddress().matches("\\d+\\.\\d+\\.\\d+\\.\\d+"));
+        // Just verify we got a valid address string - could be IPv4 or IPv6
+        String hostAddress = result.get().getHostAddress();
+        assertTrue(hostAddress != null && !hostAddress.isEmpty(), "Expected non-empty host address");
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@app/server/appsmith-interfaces/src/test/java/com/appsmith/util/WebClientUtilsTest.java`
around lines 90 - 95, The test resolveIfAllowed_returnsResolvedAddress assumes
IPv4; update it so WebClientUtils.resolveIfAllowed("smtp.gmail.com") is asserted
present and the returned InetAddress (result.get().getHostAddress()) is
validated for either IPv4 or IPv6—for example keep the existing IPv4 regex OR
accept addresses containing ':' (IPv6) or use a standard InetAddress validation
method—so the assertion passes regardless of whether the resolver returns IPv4
or IPv6.

Copy link
Copy Markdown
Collaborator Author

@subrata71 subrata71 Mar 30, 2026

Choose a reason for hiding this comment

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

@coderabbitai Should this be addressed with a code change, or does it only require updating the assertion?

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 30, 2026

Choose a reason for hiding this comment

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

@subrata71 This is only an assertion update — the production code in resolveIfAllowed() is already correct. The fix is just relaxing the test assertion so it passes whether the resolver returns an IPv4 or IPv6 address:

`@Test`
public void resolveIfAllowed_returnsResolvedAddress() {
    Optional<InetAddress> result = WebClientUtils.resolveIfAllowed("smtp.gmail.com");
    assertTrue(result.isPresent());
    String hostAddress = result.get().getHostAddress();
    assertTrue(hostAddress != null && !hostAddress.isEmpty(), "Expected non-empty host address");
}

No production code changes needed.

Copy link
Copy Markdown
Collaborator Author

@subrata71 subrata71 Mar 30, 2026

Choose a reason for hiding this comment

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

Sounds good. In that case it's safe to ignore this concern. @coderabbitai

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 30, 2026

Choose a reason for hiding this comment

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

@subrata71, understood — acknowledged and moving on. The production logic is sound either way, and the test will pass in the vast majority of CI environments. 👍


@SuppressWarnings("unchecked")
private Mono<ClientRequest> invokeRequestFilterFn(String url) throws Exception {
final Method method = WebClientUtils.class.getDeclaredMethod("requestFilterFn", ClientRequest.class);
Expand Down
54 changes: 48 additions & 6 deletions ...rver/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/EnvManagerCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.appsmith.server.services.PermissionGroupService;
import com.appsmith.server.services.SessionUserService;
import com.appsmith.server.services.UserService;
import com.appsmith.util.WebClientUtils;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import jakarta.mail.MessagingException;
Expand Down Expand Up @@ -783,18 +784,59 @@ public Mono<Void> restartWithoutAclCheck() {
return Mono.empty();
}

private static final Set<Integer> DEFAULT_SMTP_PORTS = Set.of(25, 465, 587, 2525);

private static final Set<Integer> ALLOWED_SMTP_PORTS = computeAllowedSmtpPorts();

private static Set<Integer> computeAllowedSmtpPorts() {
Set<Integer> ports = new HashSet<>(DEFAULT_SMTP_PORTS);
String extra = System.getenv("APPSMITH_MAIL_ALLOWED_PORTS");
if (extra != null && !extra.isBlank()) {
for (String token : extra.split(",")) {
try {
int port = Integer.parseInt(token.trim());
if (port > 0 && port <= 65535) {
ports.add(port);
}
} catch (NumberFormatException ignored) {
}
}
}
return Set.copyOf(ports);
}

private static final String SMTP_GENERIC_ERROR =
"Failed to connect to the SMTP server. Please verify the host, " + "port, and credentials are correct.";

@Override
public Mono<Boolean> sendTestEmail(TestEmailConfigRequestDTO requestDTO) {
return verifyCurrentUserIsSuper().flatMap(user -> {
if (!ALLOWED_SMTP_PORTS.contains(requestDTO.getSmtpPort())) {
return Mono.error(
new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration."));
}

var resolvedAddress = WebClientUtils.resolveIfAllowed(requestDTO.getSmtpHost());
if (resolvedAddress.isEmpty()) {
return Mono.error(
new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Invalid SMTP configuration."));
}

JavaMailSenderImpl mailSender = new JavaMailSenderImpl();
mailSender.setHost(requestDTO.getSmtpHost());
mailSender.setHost(resolvedAddress.get().getHostAddress());
mailSender.setPort(requestDTO.getSmtpPort());

Properties props = mailSender.getJavaMailProperties();
props.put("mail.transport.protocol", "smtp");

props.put(
"mail.smtp.starttls.enable", requestDTO.getStarttlsEnabled().toString());
if (requestDTO.getSmtpPort() == 465) {
props.put("mail.smtp.ssl.enable", "true");
props.put("mail.smtp.starttls.enable", "false");
} else {
props.put(
"mail.smtp.starttls.enable",
requestDTO.getStarttlsEnabled().toString());
}

props.put("mail.smtp.timeout", 7000); // 7 seconds

Expand All @@ -817,15 +859,15 @@ public Mono<Boolean> sendTestEmail(TestEmailConfigRequestDTO requestDTO) {
try {
mailSender.testConnection();
} catch (MessagingException e) {
return Mono.error(new AppsmithException(
AppsmithError.GENERIC_BAD_REQUEST, e.getMessage().trim()));
log.error("SMTP test-connection failed for host {}", requestDTO.getSmtpHost(), e);
return Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, SMTP_GENERIC_ERROR));
}

try {
mailSender.send(message);
} catch (MailException mailException) {
log.error("failed to send test email", mailException);
return Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, mailException.getMessage()));
return Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, SMTP_GENERIC_ERROR));
}
return Mono.just(TRUE);
});
Expand Down
49 changes: 49 additions & 0 deletions app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.appsmith.server.configurations.EmailConfig;
import com.appsmith.server.configurations.GoogleRecaptchaConfig;
import com.appsmith.server.domains.User;
import com.appsmith.server.dtos.TestEmailConfigRequestDTO;
import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.BlacklistedEnvVariableHelper;
Expand All @@ -24,6 +25,8 @@
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mockito;
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.mail.javamail.JavaMailSender;
Expand Down Expand Up @@ -384,6 +387,52 @@ public void sendTestEmail_WhenUserNotSuperUser_ThrowsException() {
.verify();
}

private void mockSuperUser() {
User user = new User();
user.setEmail("admin@appsmith.com");
Mockito.when(userUtils.isCurrentUserSuperUser()).thenReturn(Mono.just(true));
Mockito.when(sessionUserService.getCurrentUser()).thenReturn(Mono.just(user));
}

private TestEmailConfigRequestDTO buildDto(String smtpHost) {
return buildDto(smtpHost, 25);
}

private TestEmailConfigRequestDTO buildDto(String smtpHost, int port) {
TestEmailConfigRequestDTO dto = new TestEmailConfigRequestDTO();
dto.setSmtpHost(smtpHost);
dto.setSmtpPort(port);
dto.setFromEmail("test@appsmith.com");
dto.setStarttlsEnabled(false);
return dto;
}

@ParameterizedTest
@ValueSource(strings = {"127.0.0.1", "169.254.169.254", "localhost"})
public void sendTestEmail_WhenBlockedHost_ThrowsException(String host) {
mockSuperUser();

StepVerifier.create(envManager.sendTestEmail(buildDto(host)))
.expectErrorSatisfies(e -> {
assertThat(e).isInstanceOf(AppsmithException.class);
assertThat(e.getMessage()).contains("Invalid SMTP configuration");
})
.verify();
}

@ParameterizedTest
@ValueSource(ints = {80, 443, 6379, 8080, 27017, 0, -1})
public void sendTestEmail_WhenDisallowedPort_ThrowsException(int port) {
mockSuperUser();

StepVerifier.create(envManager.sendTestEmail(buildDto("smtp.gmail.com", port)))
.expectErrorSatisfies(e -> {
assertThat(e).isInstanceOf(AppsmithException.class);
assertThat(e.getMessage()).contains("Invalid SMTP configuration");
})
.verify();
}
Comment on lines +423 to +434
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This port matrix is environment-dependent.

APPSMITH_MAIL_ALLOWED_PORTS can whitelist any positive port, so hard-coding 80/443/6379/8080/27017 here makes the test depend on the runner’s environment. If one of those ports is allowed, this no longer exercises the validation branch and can fall through into DNS/network behavior instead. Please derive the negative cases from the runtime allowlist, or restrict this test to values that can never be whitelisted.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/EnvManagerTest.java`
around lines 423 - 434, The test currently uses hard-coded positive ports which
may be whitelisted via APPSMITH_MAIL_ALLOWED_PORTS and make the test flaky;
update sendTestEmail_WhenDisallowedPort_ThrowsException to avoid
environment-dependent ports by either (a) restrict the `@ValueSource` to only
values that can never be whitelisted (e.g., 0 and -1) so the validation branch
is always exercised, or (b) compute a disallowed port at runtime by reading the
configured allowlist (APPSMITH_MAIL_ALLOWED_PORTS) and selecting a port not in
that set, then call envManager.sendTestEmail(buildDto("smtp.gmail.com",
chosenPort)) within the test; adjust assertions around sendTestEmail
accordingly.


@Test
public void setEnv_AndGetAll() {
// Create a test map of environment variables
Expand Down
Loading