opnsense / opnsense/plugins

os-git-backup: SCP-style SSH URLs corrupted by credential injection

Open Beginner friendly
#5,629 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
PHP
Stars
1.2k
Forks
863
Avg merge
2d 6h
Merged PRs (30d)
10

Description

Important notices
Before you add a new report, we ask you kindly to acknowledge the following:

Describe the bug

backup() in src/opnsense/mvc/app/library/OPNsense/Backup/Git.php corrupts SCP-style SSH URLs (git@github.com:user/repo.git) when the User Name field is empty. strpos($url, '//') returns false for URLs without //, PHP evaluates false + 2 as 2, and substr() splits the URL at position 2:

git@github.com:user/repo.git
-> gi + @ + t@github.com:user/repo.git
-> gi@t@github.com:user/repo.git

git push then fails with an authentication error against the invalid remote URL.

Two defects combine here (lines 165-172):

  1. The code uses $pos in substr() without checking for false. SCP-style URLs contain no //.
  2. For non-HTTP URLs the code injects the user even when the field is empty, producing a bare @.

Last known working version: none known.

To Reproduce

  1. Install os-git-backup
  2. Set URL to git@github.com:user/repo.git
  3. Leave User Name empty
  4. Provide an SSH private key
  5. Run the backup

Expected behavior

The SCP-style URL passes through to git remote add origin unchanged; the SSH key handles authentication.

Screenshots

Not applicable.

Relevant log files

The backup log shows git push failing with an authentication error against the corrupted remote URL.

Additional context

Suggested fix: wrap credential injection in a $pos !== false check so the code never modifies SCP-style URLs, and skip injection for SSH URLs without a configured user:

if ($pos !== false) {
    if (stripos(trim((string)$mdl->url), 'http') === 0) {
        $cred = urlencode((string)$mdl->user) . ":" . urlencode((string)$mdl->password);
        $url = substr($url, 0, $pos + 2) . "{$cred}@" . substr($url, $pos + 2);
    } elseif (!empty((string)$mdl->user)) {
        $url = substr($url, 0, $pos + 2) . urlencode((string)$mdl->user) . "@" . substr($url, $pos + 2);
    }
}

Workaround: use ssh://github.com/user/repo.git with git in the User Name field; the plugin then builds ssh://git@github.com/user/repo.git.

Refiled from #5234 on the issue template; closing the original.

Environment

OPNsense 26.1.2
os-git-backup (current plugins master)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Open src/opnsense/mvc/app/library/OPNsense/Backup/Git.php and inspect backup() around lines 165-172, focusing on how strpos() and the empty User Name field affect credential injection. Reproduce with git@github.com:user/repo.git and no User Name, then verify that the remote URL passes unchanged and git push no longer uses the corrupted URL.

Written by the indexing model from the issue text.

Assessment

Tech stack
git, php
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.