Skip to content

Prevent XSS attack on log.php#224

Closed
Badatos wants to merge 1 commit intowebsvnphp:masterfrom
Badatos:master
Closed

Prevent XSS attack on log.php#224
Badatos wants to merge 1 commit intowebsvnphp:masterfrom
Badatos:master

Conversation

@Badatos
Copy link
Copy Markdown
Contributor

@Badatos Badatos commented May 16, 2025

Hi !
I discovered a potentiel vulnerability on websvn/log.php, and here is a PR correcting it.

How to reproduce

Go to https://site.domain/websvn/log.php?repname=MYREPO&path=%3C/script%3E%27%22%3E%3Cimg%20src=x%20onError=prompt(1)%3E&isdir=1

==> if you see a prompting popup, your site is vulnerable.

My change ensure the "path" variable is escaped before inserting it in DOM

@michael-o
Copy link
Copy Markdown
Member

Merci, I will happily look at it. Are there maybe other issues like this in the source code?

@michael-o michael-o self-assigned this May 16, 2025
@Badatos
Copy link
Copy Markdown
Contributor Author

Badatos commented May 16, 2025

Maybe there are others, but I did not spotted them yet ;)

@michael-o
Copy link
Copy Markdown
Member

Maybe there are others, but I did not spotted them yet ;)

Would you mind to take a look or do you want to keep this one as-is?

@michael-o
Copy link
Copy Markdown
Member

@Badatos
Copy link
Copy Markdown
Contributor Author

Badatos commented May 16, 2025

I do not have any automated tool to test all websvn pages, but none of the other hidden input seems to have the same vulnerability :

  • repname always correspond to an existing repo, or is not displayed
  • path is corrected here
  • some others already have their value escaped
  • and the last remaining just have a fixed value (manualorder, op, ...) ;)

@michael-o
Copy link
Copy Markdown
Member

michael-o commented May 16, 2025

I do not have any automated tool to test all websvn pages, but none of the other hidden input seems to have the same vulnerability :

* `repname` always correspond to an existing repo, or is not displayed

* `path` is corrected here

* some others already have their value escaped

* and the last remaining just have a fixed value (manualorder, op, ...) ;)

I agree with you. Will try to merge later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants