[ https://issues.apache.org/jira/browse/SCM-1028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17941512#comment-17941512 ]
ASF GitHub Bot commented on SCM-1028: ------------------------------------- mhoffrog commented on code in PR #237: URL: https://github.com/apache/maven-scm/pull/237#discussion_r2030806878 ########## maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/AnonymousCommandLine.java: ########## @@ -29,25 +27,14 @@ */ public class AnonymousCommandLine extends Commandline { - public static final String PASSWORD_PLACE_HOLDER = "********"; - - private Pattern passwordPattern = Pattern.compile("^.*:(.*)@.*$"); - /** * Provides an anonymous output to mask password. Considering URL of type : * <<protocol>>://<<user>>:<<password>>@ * <<host_definition>> */ @Override public String toString() { - String output = super.toString(); - final Matcher passwordMatcher = passwordPattern.matcher(output); - if (passwordMatcher.find()) { Review Comment: @michael-o This is the place where the find() did have been taken from - did not want to touch this. ########## maven-scm-providers/maven-scm-providers-git/maven-scm-provider-git-commons/src/main/java/org/apache/maven/scm/provider/git/util/GitUtil.java: ########## @@ -77,4 +86,24 @@ public static void setSettingsDirectory(File directory) { public static File getSettingsFile() { return new File(settingsDirectory, GIT_SETTINGS_FILENAME); } + + /** + * Provides an anonymous output to mask password. Considering URL of type : + * <<protocol>>://<<user>>:<<password>>@ + * <<host_definition>> + * + * @param urlWithCredentials + * @return urlWithCredentials but password masked with stars + */ + public static String maskPasswordInUrl(String urlWithCredentials) { + String output = urlWithCredentials; + final Matcher passwordMatcher = PASSWORD_IN_URL_PATTERN.matcher(output); + if (passwordMatcher.find()) { + // clear password with delimiters + final String clearPasswordWithDelimiters = passwordMatcher.group(1); + // to be replaced in output by stars with delimiters Review Comment: This phrasing is being used at [ScmResult.java](https://github.com/apache/maven-scm/blob/55f3e9ec07567ea97e3c58db5d495389d48b34b9/maven-scm-api/src/main/java/org/apache/maven/scm/ScmResult.java#L117) as well. Did keep this for consistency. Shall we update it everywhere to asterisks then? ########## maven-scm-providers/maven-scm-providers-git/maven-scm-provider-git-commons/src/main/java/org/apache/maven/scm/provider/git/util/GitUtil.java: ########## @@ -77,4 +86,24 @@ public static void setSettingsDirectory(File directory) { public static File getSettingsFile() { return new File(settingsDirectory, GIT_SETTINGS_FILENAME); } + + /** + * Provides an anonymous output to mask password. Considering URL of type : + * <<protocol>>://<<user>>:<<password>>@ + * <<host_definition>> + * + * @param urlWithCredentials + * @return urlWithCredentials but password masked with stars + */ + public static String maskPasswordInUrl(String urlWithCredentials) { + String output = urlWithCredentials; + final Matcher passwordMatcher = PASSWORD_IN_URL_PATTERN.matcher(output); + if (passwordMatcher.find()) { Review Comment: That code was taken 1:1 from [AnonymousCommandLine.java](https://github.com/apache/maven-scm/blob/55f3e9ec07567ea97e3c58db5d495389d48b34b9/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/AnonymousCommandLine.java#L45) - did not want to touch this - see comment below. > Vulnerability: Clear text password is logged by JGit provider and by gitexe > remoteinfo on a ls-remote failure > ------------------------------------------------------------------------------------------------------------- > > Key: SCM-1028 > URL: https://issues.apache.org/jira/browse/SCM-1028 > Project: Maven SCM > Issue Type: Bug > Components: maven-scm-provider-gitexe, maven-scm-provider-jgit > Affects Versions: 2.1.0 > Reporter: Markus Hoffrogge > Priority: Critical > Labels: Vulnerability, vulnerabilities, vulnerability > Original Estimate: 24h > Remaining Estimate: 24h > > *Issue(s):* > # {*}JGit provider{*}: If the git password contains special characters which > are differently encoded by the {{URI class}} than {{{}by > URLEncode.encode{}}}, then the password masking does not become effective and > the password is logged in clear URI encoded format by the jgit provider. > # {*}Gitexe remoteinfo{*}: In case ls-remote is failing, then a > {{ScmException}} is being thrown with the fetch URL passed as error message > containing the URI encoded clear password. > *Root cause(s):* > # The URL encoding used for the credentials within fetch and push URL > differs from the encoding being used for masking the password at > [JGitUtils.prepareSession(...)|https://github.com/apache/maven-scm/blob/55186fdf42f65fd3a1be07161bc198f092386f77/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-jgit/src/main/java/org/apache/maven/scm/provider/git/jgit/command/JGitUtils.java#L149] > # Password is not masked for the exception message passed to the > ScmException used at > [GitRemoteInfoCommand.executeRemoteInfoCommand(...)|https://github.com/apache/maven-scm/blob/55186fdf42f65fd3a1be07161bc198f092386f77/maven-scm-providers/maven-scm-providers-git/maven-scm-provider-gitexe/src/main/java/org/apache/maven/scm/provider/git/gitexe/command/remoteinfo/GitRemoteInfoCommand.java#L59] > *Solution:* > [PR #237|https://github.com/apache/maven-scm/pull/237] -- This message was sent by Atlassian Jira (v8.20.10#820010)