Skip to content

Commit

Permalink
fix Ambari Spoofing Vulnerability
Browse files Browse the repository at this point in the history
  • Loading branch information
brahmareddybattula committed Nov 29, 2023
1 parent f2a21c8 commit aa531af
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 19 deletions.
27 changes: 16 additions & 11 deletions ambari-common/src/main/python/resource_management/core/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,15 @@ def checked_call(command, quiet=False, logoutput=None, stdout=subprocess32.PIPE,
@log_function_call
def call(command, quiet=False, logoutput=None, stdout=subprocess32.PIPE,stderr=subprocess32.STDOUT,
cwd=None, env=None, preexec_fn=preexec_fn, user=None, wait_for_finish=True, timeout=None, on_timeout=None,
path=None, sudo=False, on_new_line=None, tries=1, try_sleep=0, timeout_kill_strategy=TerminateStrategy.TERMINATE_PARENT, returns=[0]):
path=None, sudo=False, on_new_line=None, tries=1, try_sleep=0, timeout_kill_strategy=TerminateStrategy.TERMINATE_PARENT, shell=True, returns=[0]):
"""
Execute the shell command despite failures.
@return: return_code, output
"""
return _call_wrapper(command, logoutput=logoutput, throw_on_failure=False, stdout=stdout, stderr=stderr,
cwd=cwd, env=env, preexec_fn=preexec_fn, user=user, wait_for_finish=wait_for_finish,
on_timeout=on_timeout, timeout=timeout, path=path, sudo=sudo, on_new_line=on_new_line,
tries=tries, try_sleep=try_sleep, timeout_kill_strategy=timeout_kill_strategy, returns=returns)
tries=tries, try_sleep=try_sleep, timeout_kill_strategy=timeout_kill_strategy, shell=shell, returns=returns)

@log_function_call
def non_blocking_call(command, quiet=False, stdout=None, stderr=None,
Expand Down Expand Up @@ -166,7 +166,7 @@ def _call_wrapper(command, **kwargs):

def _call(command, logoutput=None, throw_on_failure=True, stdout=subprocess32.PIPE,stderr=subprocess32.STDOUT,
cwd=None, env=None, preexec_fn=preexec_fn, user=None, wait_for_finish=True, timeout=None, on_timeout=None,
path=None, sudo=False, on_new_line=None, tries=1, try_sleep=0, timeout_kill_strategy=TerminateStrategy.TERMINATE_PARENT, returns=[0]):
path=None, sudo=False, on_new_line=None, tries=1, try_sleep=0, timeout_kill_strategy=TerminateStrategy.TERMINATE_PARENT, shell=True, returns=[0]):
"""
Execute shell command
Expand Down Expand Up @@ -200,18 +200,23 @@ def _call(command, logoutput=None, throw_on_failure=True, stdout=subprocess32.PI
command = as_sudo(command, env=env)
elif user:
command = as_user(command, user, env=env)

# convert to string and escape
if isinstance(command, (list, tuple)):
command = string_cmd_from_args_list(command)


if isinstance(command, basestring):
subprocess32_command = [command]
elif shell:
# TODO: Remove this condition after reviewing 'shell' flag requirement where shell.call() is being used.
subprocess32_command = [string_cmd_from_args_list(command)]
else:
subprocess32_command = command

# replace placeholder from as_sudo / as_user if present
env_str = _get_environment_str(env)
for placeholder, replacement in PLACEHOLDERS_TO_STR.iteritems():
command = command.replace(placeholder, replacement.format(env_str=env_str))
subprocess32_command = [cmd.replace(placeholder, replacement.format(env_str=env_str)) for cmd in subprocess32_command]

# --noprofile is used to preserve PATH set for ambari-agent
subprocess32_command = ["/bin/bash","--login","--noprofile","-c", command]
if shell:
# --noprofile is used to preserve PATH set for ambari-agent
subprocess32_command = ["/bin/bash","--login","--noprofile","-c"] + subprocess32_command

# don't create stdout and stderr pipes, because forked process will not be able to use them if current process dies
# creating pipes may lead to the forked process silent crash
Expand Down
5 changes: 5 additions & 0 deletions ambari-server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1162,6 +1162,11 @@
<artifactId>commons-compress</artifactId>
<version>1.21</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>1.10.0</version>
</dependency>
<dependency>
<groupId>uk.com.robust-it</groupId>
<artifactId>cloning</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@
import org.apache.commons.lang.BooleanUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.apache.http.HttpStatus;
import org.apache.http.client.utils.URIBuilder;
import org.slf4j.Logger;
Expand Down Expand Up @@ -4084,7 +4085,7 @@ public RequestStatusResponse createAction(ExecuteActionRequest actionRequest,
String requestContext = "";

if (requestProperties != null) {
requestContext = requestProperties.get(REQUEST_CONTEXT_PROPERTY);
requestContext = StringEscapeUtils.escapeHtml4(requestProperties.get(REQUEST_CONTEXT_PROPERTY));
if (requestContext == null) {
// guice needs a non-null value as there is no way to mark this parameter @Nullable
requestContext = "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.apache.ambari.server.state.alert.AlertGroup;
import org.apache.ambari.server.state.alert.AlertTarget;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -275,7 +276,7 @@ protected Set<String> getPKPropertyIds() {
private void createAlertTargets(Set<Map<String, Object>> requestMaps, Map<String, String> requestInfoProps)
throws AmbariException {
for (Map<String, Object> requestMap : requestMaps) {
String name = (String) requestMap.get(ALERT_TARGET_NAME);
String name = StringEscapeUtils.escapeHtml4((String) requestMap.get(ALERT_TARGET_NAME));
String description = (String) requestMap.get(ALERT_TARGET_DESCRIPTION);
String notificationType = (String) requestMap.get(ALERT_TARGET_NOTIFICATION_TYPE);
Collection<String> alertStates = (Collection<String>) requestMap.get(ALERT_TARGET_STATES);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
import org.apache.ambari.server.topology.TopologyManager;
import org.apache.ambari.server.utils.SecretReference;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -431,7 +432,7 @@ private RequestRequest getRequest(Map<String, Object> propertyMap) {
// in this case it will be mapped to HTTP 400 Bad Request
requestStatus = HostRoleStatus.valueOf(requestStatusStr);
}
String abortReason = (String) propertyMap.get(REQUEST_ABORT_REASON_PROPERTY_ID);
String abortReason = StringEscapeUtils.escapeHtml4((String) propertyMap.get(REQUEST_ABORT_REASON_PROPERTY_ID));
String removePendingHostRequests = (String) propertyMap.get(REQUEST_REMOVE_PENDING_HOST_REQUESTS_ID);

RequestRequest requestRequest = new RequestRequest(clusterNameStr, requestId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from resource_management.core import shell
from resource_management.core.logger import Logger
from resource_management.core.utils import PasswordString
from shlex import split


# WARNING. If you are adding a new host check that is used by cleanup, add it to BEFORE_CLEANUP_HOST_CHECKS
Expand Down Expand Up @@ -454,7 +455,7 @@ def execute_db_connection_check(self, config, tmp_dir):
db_connection_check_command = "LD_LIBRARY_PATH=$LD_LIBRARY_PATH:{0}{1} {2}".format(agent_cache_dir,
LIBS_PATH_IN_ARCHIVE_SQLA, db_connection_check_command)

code, out = shell.call(db_connection_check_command)
code, out = shell.call(split(db_connection_check_command, comments=True), shell=False)

if code == 0:
db_connection_check_structured_output = {"exit_code" : 0, "message": "DB connection check completed successfully!" }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ App.DiffTooltipComponent = Em.Component.extend({

caption += fmtString.fmt(
[prefix,item].compact().join('.'),
(oldV != null && '\'%@\''.fmt(oldV)) || emptyValue,
(newV != null && '\'%@\''.fmt(newV)) || emptyValue
Handlebars.Utils.escapeExpression((oldV != null && '\'%@\''.fmt(oldV))) || emptyValue,
Handlebars.Utils.escapeExpression((newV != null && '\'%@\''.fmt(newV))) || emptyValue
);
},
initialLabels,
Expand All @@ -79,7 +79,7 @@ App.DiffTooltipComponent = Em.Component.extend({
oldV = ((isAllChanged && changes._accessAllLabels.objectAt(0)) || (queue.get('accessAllLabels') && !isAllChanged))?'*':initialLabels.map(idsToNames).join(',') || emptyValue;
newV = ((isAllChanged && changes._accessAllLabels.objectAt(1)) || (queue.get('accessAllLabels') && !isAllChanged))?'*':currentLabels.map(idsToNames).join(',') || emptyValue;

caption += fmtString.fmt('accessible-node-labels', oldV, newV);
caption += fmtString.fmt('accessible-node-labels', Handlebars.Utils.escapeExpression(oldV), Handlebars.Utils.escapeExpression(newV));
}

queue.get('labels').forEach(function (label) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
Ember.Handlebars.helper('escapeACL', function(value) {
var output = '';

value = value || '';
value = Handlebars.Utils.escapeExpression(value || '');

if (value.trim() == '') {
output = '<span class="label label-danger"> <i class="fa fa-ban fa-fw"></i> Nobody </span> ';
Expand Down

1 comment on commit aa531af

@ysrore
Copy link

@ysrore ysrore commented on aa531af Mar 26, 2024

Choose a reason for hiding this comment

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

@brahmareddybattula I have applied this patch and build ambari when I try to install ambari I got this error on hive database connection check progress
Traceback (most recent call last): File "/var/lib/ambari-agent/cache/custom_actions/scripts/check_host.py", line 548, in <module> CheckHost().execute() File "/usr/lib/ambari-agent/lib/resource_management/libraries/script/script.py", line 352, in execute method(env) File "/var/lib/ambari-agent/cache/custom_actions/scripts/check_host.py", line 209, in actionexecute raise Fail(error_message) resource_management.core.exceptions.Fail: Check db_connection_check was unsuccessful. Exit code: 1. Message: 'tuple' object has no attribute 'read'

Please sign in to comment.