[
https://issues.apache.org/jira/browse/SCM-1028?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17940034#comment-17940034
]
ASF GitHub Bot commented on SCM-1028:
-------------------------------------
mhoffrog commented on code in PR #237:
URL: https://github.com/apache/maven-scm/pull/237#discussion_r2022735104
##########
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 +89,52 @@ public static void setSettingsDirectory(File directory) {
public static File getSettingsFile() {
return new File(settingsDirectory, GIT_SETTINGS_FILENAME);
}
+
+ /**
+ * Encodes the passed String as UTF-8 using an algorithm that's
+ * compatible with JavaScript's <code>encodeURIComponent</code> function.
+ * Returns <code>null</code> if the String is <code>null</code>.
+ * See
http://stackoverflow.com/questions/607176/java-equivalent-to-javascripts-encodeuricomponent-that-produces-identical-output
+ *
+ * @author John Topley
+ * @param s The String to be encoded
+ * @return the encoded String
+ */
+ public static String encodeURIComponent(String s) {
+ String result = s;
+ if (s != null) {
+ try {
+ result = URLEncoder.encode(s, StandardCharsets.UTF_8.name())
// nl
Review Comment:
It is for forms but it encodes more special chars as well as probable
unicode chars properly. If you look to the utility method in total it does some
adjustments in addition to comply fully with its Javascript pendent. I would
prefer for the SCM user:password encoding over the plain URI encoding. If you
insist on URI encoding for other reasons I can change it.
##########
maven-scm-providers/maven-scm-providers-git/maven-scm-provider-git-commons/src/main/java/org/apache/maven/scm/provider/git/repository/GitScmProviderRepository.java:
##########
@@ -246,21 +259,11 @@ private String getUrl(RepositoryUrl repoUrl) {
}
if (userName != null && userName.length() > 0) {
- String userInfo = userName;
+ String userInfo = GitUtil.encodeURIComponent(userName);
if (password != null && password.length() > 0) {
- userInfo += ":" + password;
+ userInfo += ":" + GitUtil.encodeURIComponent(password);
Review Comment:
It is not an improvement - it is key to get the password URL encoded. The
only change is that it is using the central util method for encoding now. The
inconsistent encoding before was the root cause of the vulnerability issue.
> 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)
