git724/forgejo
1
0
Fork 0
Code Issues Pull requests 3 Projects 1 Releases Packages Wiki Activity

fix: do not allow SSH url for migration (#7004)

Browse source 
- Add a new function `IsPushMirrorURLAllowed` that will allow `ssh://` url and make the existing `IsMigrateURLAllowed` not allow such URLs anymore.
- Resolves forgejo/forgejo#6960
- Existing integration tests make sure that SSH urls are still allowed for the push mirror feature and added unit test to ensure that `IsMigrateURLAllowed` no longer allows SSH urls.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/7004
Reviewed-by: 0ko <0ko@noreply.codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-02-21 07:57:06 +00:00 • committed by Earl Warren
commit e8ebb5d6e3
4 changed files with 28 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

2
routers/api/v1/repo/mirror.go
View file

@ -363,7 +363,7 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro
address, err := forms.ParseRemoteAddr(mirrorOption.RemoteAddress, mirrorOption.RemoteUsername, mirrorOption.RemotePassword)
if err == nil {
err = migrations.IsMigrateURLAllowed(address, ctx.ContextUser)
err = migrations.IsPushMirrorURLAllowed(address, ctx.ContextUser)
}
if err != nil {
HandleRemoteAddressError(ctx, err)

2
routers/web/repo/setting/setting.go
View file

@ -651,7 +651,7 @@ func SettingsPost(ctx *context.Context) {
address, err := forms.ParseRemoteAddr(form.PushMirrorAddress, form.PushMirrorUsername, form.PushMirrorPassword)
if err == nil {
err = migrations.IsMigrateURLAllowed(address, ctx.Doer)
err = migrations.IsPushMirrorURLAllowed(address, ctx.Doer)
}
if err != nil {
ctx.Data["Err_PushMirrorAddress"] = true

13
services/migrations/migrate.go
View file

@ -39,8 +39,17 @@ func RegisterDownloaderFactory(factory base.DownloaderFactory) {
factories = append(factories, factory)
}
// IsMigrateURLAllowed checks if an URL is allowed to be migrated from
// IsPushMirrorURLAllowed checks if an URL is allowed to be pushed to.
func IsPushMirrorURLAllowed(remoteURL string, doer *user_model.User) error {
return isURLAllowed(remoteURL, doer, true)
}
// IsMigrateURLAllowed checks if an URL is allowed to be migrated from.
func IsMigrateURLAllowed(remoteURL string, doer *user_model.User) error {
return isURLAllowed(remoteURL, doer, false)
}
func isURLAllowed(remoteURL string, doer *user_model.User, isPushMirror bool) error {
// Remote address can be HTTP/HTTPS/Git URL or local path.
u, err := url.Parse(remoteURL)
if err != nil {
@ -71,7 +80,7 @@ func IsMigrateURLAllowed(remoteURL string, doer *user_model.User) error {
return &models.ErrInvalidCloneAddr{Host: u.Host, IsURLError: true}
}
if u.Opaque != "" || u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" && u.Scheme != "git" && u.Scheme != "ssh" {
if u.Opaque != "" || u.Scheme != "" && u.Scheme != "http" && u.Scheme != "https" && u.Scheme != "git" && u.Scheme != "ssh" || (!isPushMirror && u.Scheme == "ssh") {
return &models.ErrInvalidCloneAddr{Host: u.Host, IsProtocolInvalid: true, IsPermissionDenied: true, IsURLError: true}
}

15
services/migrations/migrate_test.go
View file

@ -113,3 +113,18 @@ func TestAllowBlockList(t *testing.T) {
// reset
init("", "", false)
}
func TestURLAllowedSSH(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user2"})
sshURL := "ssh://git@git.gay/gitgay/forgejo"
t.Run("Migrate URL", func(t *testing.T) {
require.Error(t, IsMigrateURLAllowed(sshURL, user))
})
t.Run("Pushmirror URL", func(t *testing.T) {
require.NoError(t, IsPushMirrorURLAllowed(sshURL, user))
})
}