Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple Cross-Site Scripting (XSS) Vulnerabilities #317

Open
1s1ldur opened this issue Jul 22, 2024 · 1 comment
Open

Multiple Cross-Site Scripting (XSS) Vulnerabilities #317

1s1ldur opened this issue Jul 22, 2024 · 1 comment
Labels

Comments

@1s1ldur
Copy link

1s1ldur commented Jul 22, 2024

Hi!

During the penetration testing of DokuWiki, i've identified some vulnerabilities. These vulnerabilities are primarily related to Cross-Site Scripting (XSS) – which would be the A03:2021 – Injection by the OWASP top 10. Those vulnerabilities can lead to stored Cross-Site Scripting (XSS) attacks (due to the nature of DokuWiki architecture), which are a serious security concern.

So let's dive in.

The fill method within indexmenu.js is designed to load and display child nodes when they're not already available. This method retrieves node data through AJAX and dynamically updates the DOM. However, it fails to sanitize the node data before inserting it into the page.
To sanitize this vulnerability i suggest that any content that's added to the DOM is sanitized or using safer methods that do not involve direct HTML insertion.

Next,
the contextmenu method within indexmenu.js shows a context menu when you right-click on a node, it constructs HTML for the menu and inserts it into the DOM, but does not sanitize the node names or other content that could be used for XSS attacks. Again to remediate this vulnerability i would suggest sanitizing the node name or using safer DOM manipulation methods that do not involve injecting HTML directly.

Last but not least,
the arrconcat function within contextmenu.js is responsible for filling the context menu with entries based on configuration arrays, by generating HTML for each menu item and inserting it into the context menu. The problem lies in how the arrconcat function handles user-supplied or dynamically generated data without proper sanitization. Specifically, the function constructs HTML anchor tags by concatenating strings, including untrusted data. The use of eval(entry[1]) to evaluate the content of entry[1] as JavaScript code is dangerous and unsafe, so this method allows any injected script within entry[1] to execute which is the root problem. Additionally, the function inserts HTML content directly into the DOM by using innerHTML. This practice does not sanitize the input, enabling any included script tags or event handlers to execute, which was the case i observed during the penetration test.

@Klap-in
Copy link
Collaborator

Klap-in commented Jul 23, 2024

Thanks for reporting your observations!
At first sight these appear to affect mostly the older part of the code, which is due for replacement with recent introduced variant of the tree. Challenge for me is lack of time as I have a newborn which needs a lot of time.
I will see how I can address these concerns on short term.
Thanks for the report with background.

@Klap-in Klap-in added the bug label Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants