From 9f955b300b11c64f79ef6a77c92fd6e1226eeb5a Mon Sep 17 00:00:00 2001 From: Gusted Date: Thu, 21 Aug 2025 02:07:50 +0200 Subject: [PATCH] fix: don't allow credentials in migrate/push mirror URL Do not allow credentials to be present in the URLs that are provided for migrations and push mirrors. They have to be given via the dedicated input fields. Give a error when this happens. There's nothing wrong with trying have the backend "correct" this, but would be a larger patch than necessary in the context of a security fix. This can be done in public. --- models/error.go | 4 ++++ options/locale_next/locale_en-US.json | 1 + routers/api/v1/repo/migrate.go | 2 ++ routers/api/v1/repo/mirror.go | 2 ++ routers/web/repo/migrate.go | 2 ++ routers/web/repo/setting/setting.go | 2 ++ services/forms/repo_form.go | 3 +++ 7 files changed, 16 insertions(+) diff --git a/models/error.go b/models/error.go index ebaa8a135d..99c8ded766 100644 --- a/models/error.go +++ b/models/error.go @@ -121,6 +121,7 @@ type ErrInvalidCloneAddr struct { IsInvalidPath bool IsProtocolInvalid bool IsPermissionDenied bool + HasCredentials bool LocalPath bool } @@ -143,6 +144,9 @@ func (err *ErrInvalidCloneAddr) Error() string { if err.IsURLError { return fmt.Sprintf("migration/cloning from '%s' is not allowed: the provided url is invalid", err.Host) } + if err.HasCredentials { + return fmt.Sprintf("migration/cloning from '%s' is not allowed: the provided url contains credentials", err.Host) + } return fmt.Sprintf("migration/cloning from '%s' is not allowed", err.Host) } diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index c897ad7ff2..9d39f40558 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -54,6 +54,7 @@ "other": "wants to merge %[1]d commits from %[2]s into %[3]s" }, "repo.form.cannot_create": "All spaces in which you can create repositories have reached the limit of repositories.", + "migrate.form.error.url_credentials": "The URL contains contains credentials, put them in the username and password fields respectively", "repo.issue_indexer.title": "Issue Indexer", "search.milestone_kind": "Search milestones…", "repo.settings.push_mirror.branch_filter.label": "Branch filter (optional)", diff --git a/routers/api/v1/repo/migrate.go b/routers/api/v1/repo/migrate.go index a848a950db..e58545c2f6 100644 --- a/routers/api/v1/repo/migrate.go +++ b/routers/api/v1/repo/migrate.go @@ -283,6 +283,8 @@ func handleRemoteAddrError(ctx *context.APIContext, err error) { } case addrErr.IsInvalidPath: ctx.Error(http.StatusUnprocessableEntity, "", "Invalid local path, it does not exist or not a directory.") + case addrErr.HasCredentials: + ctx.Error(http.StatusUnprocessableEntity, "", "The URL contains credentials.") default: ctx.Error(http.StatusInternalServerError, "ParseRemoteAddr", "Unknown error type (ErrInvalidCloneAddr): "+err.Error()) } diff --git a/routers/api/v1/repo/mirror.go b/routers/api/v1/repo/mirror.go index 08ef68cbfc..fa2d5abb11 100644 --- a/routers/api/v1/repo/mirror.go +++ b/routers/api/v1/repo/mirror.go @@ -442,6 +442,8 @@ func HandleRemoteAddressError(ctx *context.APIContext, err error) { ctx.Error(http.StatusBadRequest, "CreatePushMirror", "Invalid Url ") case addrErr.IsPermissionDenied: ctx.Error(http.StatusUnauthorized, "CreatePushMirror", "Permission denied") + case addrErr.HasCredentials: + ctx.Error(http.StatusBadRequest, "CreatePushMirror", "The URL contains credentials") default: ctx.Error(http.StatusBadRequest, "CreatePushMirror", "Unknown error") } diff --git a/routers/web/repo/migrate.go b/routers/web/repo/migrate.go index 86d2461e94..3a5cf30dbe 100644 --- a/routers/web/repo/migrate.go +++ b/routers/web/repo/migrate.go @@ -138,6 +138,8 @@ func handleMigrateRemoteAddrError(ctx *context.Context, err error, tpl base.TplN } case addrErr.IsInvalidPath: ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tpl, form) + case addrErr.HasCredentials: + ctx.RenderWithErr(ctx.Tr("migrate.form.error.url_credentials"), tpl, form) default: log.Error("Error whilst updating url: %v", err) ctx.RenderWithErr(ctx.Tr("form.url_error", "unknown"), tpl, form) diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 59e34baf1b..ec18fa55a9 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -1116,6 +1116,8 @@ func handleSettingRemoteAddrError(ctx *context.Context, err error, form *forms.R } case addrErr.IsInvalidPath: ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_local_path"), tplSettingsOptions, form) + case addrErr.HasCredentials: + ctx.RenderWithErr(ctx.Tr("migrate.form.error.url_credentials"), tplSettingsOptions, form) default: ctx.ServerError("Unknown error", err) } diff --git a/services/forms/repo_form.go b/services/forms/repo_form.go index d040b41395..11aac1fd52 100644 --- a/services/forms/repo_form.go +++ b/services/forms/repo_form.go @@ -105,6 +105,9 @@ func ParseRemoteAddr(remoteAddr, authUsername, authPassword string) (string, err if err != nil { return "", &models.ErrInvalidCloneAddr{IsURLError: true, Host: remoteAddr} } + if u.User != nil { + return "", &models.ErrInvalidCloneAddr{Host: remoteAddr, HasCredentials: true} + } if len(authUsername)+len(authPassword) > 0 { u.User = url.UserPassword(authUsername, authPassword) }