Klap-in
commented
4 months ago 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.
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
fillmethod withinindexmenu.jsis 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
contextmenumethod withinindexmenu.jsshows 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
arrconcatfunction withincontextmenu.jsis 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 thearrconcatfunction 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 ofeval(entry[1])to evaluate the content ofentry[1]as JavaScript code is dangerous and unsafe, so this method allows any injected script withinentry[1]to execute which is the root problem. Additionally, the function inserts HTML content directly into the DOM by usinginnerHTML. 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.