fix: services uncontrolled data used in path expression

[opsysdebug]

fix the problem, we should ensure that the resolved path is strictly contained within the intended root directory of the file server. This can be done by resolving the user-supplied path against the root, normalizing it, and then checking that the resulting absolute path is still within the root directory. In the context of Go's http.FileSystem, this means we should check that the cleaned path does not escape the root of the embedded filesystem. Since we may not have access to the actual filesystem path (e.g., with embed.FS), we can instead reject any path that, after cleaning, contains any parent directory references (..) or is absolute. We should also ensure that the path is not empty and does not contain any path separators at the start (other than the leading slash).

The best way to implement this is to add a check after cleaning the path, before opening the file, to ensure that the path is relative and does not contain any .. components. This can be done by splitting the path and checking its components. The changes should be made in the ServeHTTP method in services/web/pkg/assets/server.go, specifically after line 81 and before line 99.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mklos-kw]

Thanks for posting the PR. This is a know issue since: #11483
Implementation with checking base path at the level of path string argument was already tried but 1. tests fail same as in this PR; 2. there is a more secure approach possible.

[opsysdebug]

@mklos-kw Thank you for the feedback. I understand that the previous implementation failed the same tests. In this new pull request, I’ve taken a different direction by focusing on a more secure approach as suggested, aiming to address the issue in a more reliable way.

Best regards,
@opsysdebug