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

[cometvisu] Security fixes & cleanup for cometvisu backend #2671

Merged
merged 12 commits into from
Jul 17, 2024

Conversation

peuter
Copy link
Member

@peuter peuter commented Jul 14, 2024

add required authentication for some rest endpoints, add some sanity checks to improve security.

Remove code that has been marked as deprecated.

peuter added 11 commits April 4, 2023 19:53
wrong backup/trash file creation

Signed-off-by: Tobias Bräutigam <[email protected]>
Signed-off-by: Tobias Bräutigam <[email protected]>
Signed-off-by: Tobias Bräutigam <[email protected]>
Signed-off-by: Tobias Bräutigam <[email protected]>
check if the file has been delivered by openHAB

Signed-off-by: Tobias Bräutigam <[email protected]>
remove handling of old hidden file syntax
improve written code

Signed-off-by: Tobias Bräutigam <[email protected]>
errors to the definition

Signed-off-by: Tobias Bräutigam <[email protected]>
@peuter peuter mentioned this pull request Jul 14, 2024
@peuter peuter requested a review from a team July 17, 2024 17:45
@peuter peuter changed the title Security fixes & cleanup for cometvisu backend [cometvisu] Security fixes & cleanup for cometvisu backend Jul 17, 2024
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @peuter for the fixes and the clean up!

@kaikreuzer kaikreuzer merged commit 2056367 into openhab:main Jul 17, 2024
5 checks passed
@kaikreuzer kaikreuzer added the bug Something isn't working label Jul 17, 2024
@kaikreuzer kaikreuzer added this to the 4.3 milestone Jul 17, 2024
@florian-h05
Copy link
Contributor

Thanks for the security fixed @peuter - do you mind taking a look at the code scanning results for CometVisu?
https://github.com/openhab/openhab-webui/security/code-scanning?query=is%3Aopen+branch%3Amain+language%3Ajava (you need to check what's for CometVisu, I haven't found a way to filter that list).

@peuter
Copy link
Member Author

peuter commented Jul 18, 2024

I will take a look at the code scanning results, will take some time because there are quite a lot of them. I just randomly looked into some of them and they looked like false positives (or negatives regarding the topic ;-) to me. All the "uncontrolled data used in path expression" do not seem to recognize that there is a path check in MountedFile.

But I will check that with a little bit more time than I have right now. Maybe I am missing something.

@florian-h05
Copy link
Contributor

Code scanning results for Vue/JS also had some false positives, but overall it caught a few things really well.

@kaikreuzer
Copy link
Member

@peuter Just in case you again didn't receive any notification - could you please have a look at the feedback at https://github.com/openhab/openhab-webui/security/advisories/GHSA-v7gr-mqpj-wwh3#advisory-comment-106507 and respond to it? As soon as everything is clarified, I would like to backport the fix(es) to 4.1.x and publish the advisories. Thanks!

@peuter
Copy link
Member Author

peuter commented Jul 31, 2024

No I haven't been noticed, I dont know why I do not get noticed when I am mentioned in the advisories, I checked all settings I could find in github. Anyways I will check them soon.

kaikreuzer pushed a commit to kaikreuzer/openhab-webui that referenced this pull request Aug 4, 2024
)

add required authentication for some rest endpoints, add some sanity
checks to improve security.

Remove code that has been marked as deprecated.

---------

Signed-off-by: Tobias Bräutigam <[email protected]>
@kaikreuzer kaikreuzer added the patch A PR that has been cherry-picked to a patch release branch label Aug 4, 2024
@kaikreuzer
Copy link
Member

Cherry-picked to 4.2.x.

@florian-h05 florian-h05 added the cometvisu ui Cometvisu UI label Aug 4, 2024
@kaikreuzer
Copy link
Member

@peuter Did you find the time to look into the remaining file separator issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cometvisu ui Cometvisu UI patch A PR that has been cherry-picked to a patch release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants