[ 
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)

Reply via email to