yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2026-01-06 12:36:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix snyk's reported vulnerabilities (#21167)

Browse Source 
* Fix snyk's reported vulnerabilities

* Fix tests

* Fix tests

* fix PolicyEvaluatorTest

* replace with single line imports

* Fix tests

---------

Co-authored-by: Harsha <harsha@Harshas-MacBook-Air.local>
Co-authored-by: sonikashah <sonikashah94@gmail.com>
This commit is contained in:
Sriharsha Chintalapani 2025-05-16 21:20:06 -07:00 committed by GitHub
parent 194bac6e2c
commit 4c267f8af5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 455 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/changeEvent/generic/GenericPublisher.java
View File

@ -57,6 +57,13 @@ public class GenericPublisher implements Destination<ChangeEvent> {
this.eventSubscription = eventSubscription;
this.subscriptionDestination = subscriptionDestination;
this.webhook = JsonUtils.convertValue(subscriptionDestination.getConfig(), Webhook.class);
// Validate webhook URL to prevent SSRF
if (this.webhook != null && this.webhook.getEndpoint() != null) {
org.openmetadata.service.util.URLValidator.validateURL(
this.webhook.getEndpoint().toString());
}
this.client =
getClient(subscriptionDestination.getTimeout(), subscriptionDestination.getReadTimeout());
} else {

12
openmetadata-service/src/main/java/org/openmetadata/service/resources/events/subscription/EventSubscriptionMapper.java
View File

@ -41,8 +41,18 @@ public class EventSubscriptionMapper
}
private String validateConsumerClass(String className) {
// Validate that the class belongs to our application package
if (!className.startsWith("org.openmetadata.")) {
throw new BadRequestException("Only classes from org.openmetadata package are allowed");
}
try {
Class.forName(className).asSubclass(AbstractEventConsumer.class);
// Check if the class exists and is a subclass of AbstractEventConsumer
Class<?> clazz = Class.forName(className);
if (!AbstractEventConsumer.class.isAssignableFrom(clazz)) {
throw new BadRequestException(
"Class must be a subclass of AbstractEventConsumer: " + className);
}
return className;
} catch (ClassNotFoundException e) {
throw new BadRequestException("Consumer class not found: " + className);

6
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/CompiledRule.java
View File

@ -40,6 +40,10 @@ public class CompiledRule extends Rule {
if (condition == null) {
return null;
}
// Apply safety validation before parsing expressions
ExpressionValidator.validateExpressionSafety(condition);
try {
return EXPRESSION_PARSER.parseExpression(condition);
} catch (Exception exception) {
@ -53,6 +57,8 @@ public class CompiledRule extends Rule {
if (condition == null) {
return;
}
// parseExpression already includes safety validation
Expression expression = parseExpression(condition);
RuleEvaluator ruleEvaluator = new RuleEvaluator();
SimpleEvaluationContext context =

157
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ExpressionValidator.java Normal file
View File

@ -0,0 +1,157 @@
/*
* Copyright 2021 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openmetadata.service.security.policyevaluator;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import lombok.extern.slf4j.Slf4j;
import org.openmetadata.schema.Function;
/**
* Utility class for validating SpEL expressions to prevent code injection.
*/
@Slf4j
public class ExpressionValidator {
// Cache of allowed function names from RuleEvaluator class
private static final Set<String> ALLOWED_FUNCTIONS = initAllowedFunctions();
// Patterns that indicate potentially dangerous expressions
private static final List<String> DANGEROUS_PATTERNS =
Arrays.asList(
"T(",
"new ",
"System.",
"Runtime",
"getClass",
"ClassLoader",
"forName",
"exec",
"eval",
"ProcessBuilder",
"java.lang.reflect");
private static Set<String> initAllowedFunctions() {
Set<String> allowedFunctions = new HashSet<>();
try {
// Classes that provide functions for policy expressions
List<Class<?>> evaluatorClasses =
Arrays.asList(
RuleEvaluator.class,
Class.forName("org.openmetadata.service.events.subscription.AlertsRuleEvaluator"));
for (Class<?> evaluatorClass : evaluatorClasses) {
scanClassForFunctions(evaluatorClass, allowedFunctions);
}
LOG.info("Initialized {} allowed functions for policy expressions", allowedFunctions.size());
} catch (Exception e) {
LOG.error("Failed to initialize allowed functions", e);
// Fallback to hardcoded list if reflection fails
allowedFunctions.addAll(
Arrays.asList(
"noOwner",
"isOwner",
"hasDomain",
"matchAllTags",
"matchAnyTag",
"matchAnyCertification",
"matchTeam",
"inAnyTeam",
"hasAnyRole",
"matchAnyEventType",
"matchAnyFieldChange",
"matchAnySource",
"matchUpdatedBy",
"matchAnyOwnerName",
"matchAnyEntityFqn",
"matchAnyEntityId",
"matchTestResult",
"filterByTableNameTestCaseBelongsTo",
"getTestCaseStatusIfInTestSuite",
"matchIngestionPipelineState",
"matchPipelineState",
"matchAnyDomain",
"matchConversationUser"));
LOG.info("Using fallback list of {} allowed functions", allowedFunctions.size());
}
return allowedFunctions;
}
private static void scanClassForFunctions(Class<?> clazz, Set<String> allowedFunctions) {
try {
for (Method method : clazz.getDeclaredMethods()) {
if (method.isAnnotationPresent(Function.class)) {
Function annotation = method.getAnnotation(Function.class);
allowedFunctions.add(annotation.name());
LOG.debug("Added allowed function from {}: {}", clazz.getSimpleName(), annotation.name());
}
}
} catch (Exception e) {
LOG.warn("Failed to scan functions from class {}", clazz.getName(), e);
}
}
public static void validateExpressionSafety(String expression) {
if (expression == null || expression.trim().isEmpty()) {
return;
}
// Check for dangerous patterns
for (String pattern : DANGEROUS_PATTERNS) {
if (expression.contains(pattern)) {
throw new IllegalArgumentException(
"Expression contains potentially unsafe pattern: "
+ pattern
+ ". "
+ "Only use approved policy functions with @Function annotations.");
}
}
// Extract function calls from the expression
Pattern functionPattern = Pattern.compile("\\b([a-zA-Z0-9_]+)\\s*\\(");
Matcher matcher = functionPattern.matcher(expression);
List<String> foundFunctions = new ArrayList<>();
while (matcher.find()) {
String functionName = matcher.group(1);
// Skip empty function names and logical operators
if (!functionName.isEmpty()
&& !functionName.equals("and")
&& !functionName.equals("or")
&& !functionName.equals("not")) {
foundFunctions.add(functionName);
// Check if function is allowed
if (!ALLOWED_FUNCTIONS.contains(functionName)) {
throw new IllegalArgumentException(
"Function '"
+ functionName
+ "' is not allowed in policy expressions. "
+ "Only use approved functions with @Function annotations in evaluator classes.");
}
}
}
LOG.debug("Validated expression contains only allowed functions: {}", foundFunctions);
}
public static Set<String> getAllowedFunctions() {
return new HashSet<>(ALLOWED_FUNCTIONS);
}
}

17
openmetadata-service/src/main/java/org/openmetadata/service/util/SubscriptionUtil.java
View File

@ -290,7 +290,19 @@ public class SubscriptionUtil {
default -> null;
};
if (webhookConfig != null && !CommonUtil.nullOrEmpty(webhookConfig.getEndpoint())) {
return Optional.of(webhookConfig.getEndpoint().toString());
String endpointStr = webhookConfig.getEndpoint().toString();
// Validate URL before returning
try {
URLValidator.validateURL(endpointStr);
return Optional.of(endpointStr);
} catch (Exception e) {
LOG.warn(
"[GetWebhookUrlsFromProfile] Invalid webhook URL for owner with id {} type {}: {}",
id,
entityType,
e.getMessage());
return Optional.empty();
}
} else {
LOG.debug(
"[GetWebhookUrlsFromProfile] Owner with id {} type {}, will not get any Notification as not webhook config is missing for type {}, webhookConfig {} ",
@ -422,6 +434,9 @@ public class SubscriptionUtil {
}
public static Invocation.Builder appendHeadersToTarget(Client client, String uri) {
// Validate the URI to prevent SSRF attacks
URLValidator.validateURL(uri);
Map<String, String> authHeaders = SecurityUtil.authHeaders("admin@open-metadata.org");
return SecurityUtil.addHeaders(client.target(uri), authHeaders);
}

69
openmetadata-service/src/main/java/org/openmetadata/service/util/URLValidator.java Normal file
View File

@ -0,0 +1,69 @@
/*
* Copyright 2021 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openmetadata.service.util;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Pattern;
import javax.ws.rs.BadRequestException;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
/**
* Utility class for validating URLs to prevent SSRF attacks.
*/
@Slf4j
public class URLValidator {
private static final List<String> ALLOWED_SCHEMES = Arrays.asList("http", "https");
private static final Pattern PRIVATE_IP_PATTERN =
Pattern.compile(
"^(127\\.|10\\.|172\\.(1[6-9]|2[0-9]|3[0-1])\\.|192\\.168\\.|169\\.254\\.|::1|[fF][cCdD]|[fF][eE][80-9a-fA-F]:).*");
public static void validateURL(String urlString) {
if (urlString == null || urlString.trim().isEmpty()) {
throw new BadRequestException("URL cannot be empty");
}
String host = getString(urlString);
if (PRIVATE_IP_PATTERN.matcher(host).matches()) {
throw new BadRequestException("URL targeting private/internal network not allowed");
}
}
private static @NotNull String getString(String urlString) {
URL url;
try {
URI uri = new URI(urlString);
url = uri.toURL();
} catch (URISyntaxException | MalformedURLException e) {
try {
url = new URL(urlString);
} catch (MalformedURLException ex) {
throw new BadRequestException("Invalid URL format: " + ex.getMessage());
}
}
String protocol = url.getProtocol().toLowerCase();
if (!ALLOWED_SCHEMES.contains(protocol)) {
throw new BadRequestException("URL scheme not allowed: " + protocol);
}
return url.getHost().toLowerCase();
}
}

4
openmetadata-service/src/test/java/org/openmetadata/service/resources/policies/PolicyResourceTest.java
View File

@ -269,10 +269,6 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
// isOwner() has Unexpected input parameter
failsToEvaluate(policyName, "!isOwner('unexpectedParam')");
// Invalid function name
failsToEvaluate(policyName, "invalidFunction()");
failsToEvaluate(policyName, "isOwner() || invalidFunction()");
// Invalid text
failsToEvaluate(policyName, "a");
failsToEvaluate(policyName, "abc");

147
openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/ExpressionValidatorTest.java Normal file
View File

@ -0,0 +1,147 @@
/*
* Copyright 2021 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.openmetadata.service.security.policyevaluator;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.util.Set;
import org.junit.jupiter.api.Test;
public class ExpressionValidatorTest {
@Test
void testValidExpressions() {
// Test valid expressions
String[] validExpressions = {
"noOwner()",
"!noOwner()",
"isOwner()",
"hasDomain()",
"matchAllTags('PersonalData.Personal', 'Tier.Tier1')",
"matchAnyTag('PersonalData.Personal', 'Tier.Tier1')",
"matchAnyCertification('Certification.Silver', 'Certification.Gold')",
"matchTeam()",
"inAnyTeam('marketing')",
"hasAnyRole('DataSteward', 'DataEngineer')",
"noOwner() && !isOwner()",
"noOwner() || isOwner()",
"!noOwner() && isOwner()",
"matchAllTags('PersonalData.Personal') && hasAnyRole('DataSteward')"
};
for (String expression : validExpressions) {
assertDoesNotThrow(
() -> ExpressionValidator.validateExpressionSafety(expression),
"Valid expression '" + expression + "' should not throw an exception");
}
}
@Test
void testInvalidExpressions() {
// Test expressions with disallowed patterns
String[] invalidExpressions = {
"T(java.lang.System).exit(0)",
"new java.io.File('/etc/passwd')",
"System.getProperty('user.home')",
"Runtime.getRuntime().exec('ls')",
"this.getClass().getClassLoader()",
"Class.forName('java.lang.Runtime')",
"someObject.getClass()",
"ProcessBuilder('ls').start()",
"eval('System.exit(0)')",
"java.lang.reflect.Method.invoke()"
};
for (String expression : invalidExpressions) {
Exception exception =
assertThrows(
IllegalArgumentException.class,
() -> ExpressionValidator.validateExpressionSafety(expression),
"Invalid expression '" + expression + "' should throw an IllegalArgumentException");
assertTrue(
exception.getMessage().contains("potentially unsafe pattern")
|| exception.getMessage().contains("is not allowed in policy expressions"),
"Exception message should indicate unsafe pattern or disallowed function");
}
}
@Test
void testInvalidFunctionNames() {
// Test expressions with function names not in RuleEvaluator
String[] invalidFunctions = {
"deleteAllData()",
"executeQuery('DELETE FROM users')",
"runCommand('rm -rf /')",
"customMethod()"
};
for (String expression : invalidFunctions) {
Exception exception =
assertThrows(
IllegalArgumentException.class,
() -> ExpressionValidator.validateExpressionSafety(expression),
"Expression with invalid function '"
+ expression
+ "' should throw an IllegalArgumentException");
assertTrue(
exception.getMessage().contains("is not allowed in policy expressions"),
"Exception should mention that the function is not allowed");
}
}
@Test
void testAllowedFunctionsFromRuleEvaluator() {
// Verify that the validator properly extracts all functions from RuleEvaluator
Set<String> allowedFunctions = ExpressionValidator.getAllowedFunctions();
// These functions should be present (from RuleEvaluator)
assertTrue(allowedFunctions.contains("noOwner"), "noOwner should be an allowed function");
assertTrue(allowedFunctions.contains("isOwner"), "isOwner should be an allowed function");
assertTrue(allowedFunctions.contains("hasDomain"), "hasDomain should be an allowed function");
assertTrue(
allowedFunctions.contains("matchAllTags"), "matchAllTags should be an allowed function");
assertTrue(
allowedFunctions.contains("matchAnyTag"), "matchAnyTag should be an allowed function");
assertTrue(
allowedFunctions.contains("matchAnyCertification"),
"matchAnyCertification should be an allowed function");
assertTrue(allowedFunctions.contains("matchTeam"), "matchTeam should be an allowed function");
assertTrue(allowedFunctions.contains("inAnyTeam"), "inAnyTeam should be an allowed function");
assertTrue(allowedFunctions.contains("hasAnyRole"), "hasAnyRole should be an allowed function");
// These functions should not be present
assertFalse(allowedFunctions.contains("deleteAllData"), "deleteAllData should not be allowed");
assertFalse(allowedFunctions.contains("executeQuery"), "executeQuery should not be allowed");
assertFalse(allowedFunctions.contains("runCommand"), "runCommand should not be allowed");
}
@Test
void testNullAndEmptyExpressions() {
// Test null and empty expressions (should not throw)
assertDoesNotThrow(
() -> ExpressionValidator.validateExpressionSafety(null),
"Null expression should not throw an exception");
assertDoesNotThrow(
() -> ExpressionValidator.validateExpressionSafety(""),
"Empty expression should not throw an exception");
assertDoesNotThrow(
() -> ExpressionValidator.validateExpressionSafety(" "),
"Whitespace expression should not throw an exception");
}
}

44
openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/PolicyEvaluatorTest.java
View File

@ -1,19 +1,31 @@
package org.openmetadata.service.security.policyevaluator;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.openmetadata.schema.type.MetadataOperation.ALL;
import static org.openmetadata.schema.type.MetadataOperation.CREATE;
import static org.openmetadata.schema.type.MetadataOperation.CREATE_INGESTION_PIPELINE_AUTOMATOR;
import static org.openmetadata.schema.type.MetadataOperation.DELETE;
import static org.openmetadata.schema.type.MetadataOperation.DELETE_TEST_CASE_FAILED_ROWS_SAMPLE;
import static org.openmetadata.schema.type.MetadataOperation.DEPLOY;
import static org.openmetadata.schema.type.MetadataOperation.EDIT_ALL;
import static org.openmetadata.schema.type.MetadataOperation.EDIT_CUSTOM_FIELDS;
import static org.openmetadata.schema.type.MetadataOperation.EDIT_DISPLAY_NAME;
import static org.openmetadata.schema.type.MetadataOperation.EDIT_LINEAGE;
import static org.openmetadata.schema.type.MetadataOperation.GENERATE_TOKEN;
import static org.openmetadata.schema.type.MetadataOperation.KILL;
import static org.openmetadata.schema.type.MetadataOperation.TRIGGER;
import static org.openmetadata.schema.type.MetadataOperation.VIEW_ALL;
import static org.openmetadata.schema.type.MetadataOperation.VIEW_BASIC;
import static org.openmetadata.schema.type.MetadataOperation.VIEW_QUERIES;
import static org.openmetadata.schema.type.MetadataOperation.VIEW_USAGE;
import java.util.*;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.stream.Collectors;
import org.junit.jupiter.api.Test;
import org.openmetadata.schema.type.MetadataOperation;
@ -113,8 +125,20 @@ class PolicyEvaluatorTest {
void trimPermissions_withAllowAccess_trimmed() {
List<Permission> permissions =
getPermissions(OperationContext.getAllOperations(), Access.ALLOW);
// trim works only for permissions starting with "EDIT" or "VIEW"
List<MetadataOperation> expectedOperations =
Arrays.asList(ALL, DELETE, CREATE, VIEW_ALL, EDIT_ALL);
Arrays.asList(
ALL,
DELETE,
CREATE,
CREATE_INGESTION_PIPELINE_AUTOMATOR,
VIEW_ALL,
EDIT_ALL,
DELETE_TEST_CASE_FAILED_ROWS_SAMPLE,
DEPLOY,
TRIGGER,
KILL,
GENERATE_TOKEN);
List<Permission> actual = PolicyEvaluator.trimPermissions(permissions);
assertEqualsPermissions(expectedOperations, actual);
}
@ -122,8 +146,20 @@ class PolicyEvaluatorTest {
@Test
void trimPermissions_withDenyAccess_trimmed() {
List<Permission> permissions = getPermissions(OperationContext.getAllOperations(), Access.DENY);
// trim works only for permissions starting with "EDIT" or "VIEW"
List<MetadataOperation> expectedOperations =
Arrays.asList(ALL, DELETE, CREATE, VIEW_ALL, EDIT_ALL);
Arrays.asList(
ALL,
DELETE,
CREATE,
CREATE_INGESTION_PIPELINE_AUTOMATOR,
VIEW_ALL,
EDIT_ALL,
DELETE_TEST_CASE_FAILED_ROWS_SAMPLE,
DEPLOY,
TRIGGER,
KILL,
GENERATE_TOKEN);
List<Permission> actual = PolicyEvaluator.trimPermissions(permissions);
assertEqualsPermissions(expectedOperations, actual);
}

4
pom.xml
View File

@ -101,7 +101,7 @@
<flyway.version>9.22.3</flyway.version>
<redshift-jdbc.version>2.1.0.30</redshift-jdbc.version>
<gson.version>2.11.0</gson.version>
<mysql.connector.version>9.1.0</mysql.connector.version>
<mysql.connector.version>9.3.0</mysql.connector.version>
<postgres.connector.version>42.7.4</postgres.connector.version>
<jsonschema2pojo.version>1.2.1</jsonschema2pojo.version>
<commons-lang.version>2.6</commons-lang.version>
@ -641,7 +641,7 @@
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-bom</artifactId>
<version>4.1.112.Final</version>
<version>4.1.118.Final</version>
<type>pom</type>
<scope>import</scope>
</dependency>