search

lightbeam24 / CustomTube

Choose your own YouTube layout, plus some other settings!
MIT License
109 stars 10 forks source link

XSS prevention hardening (Return YouTube Dislike) #188

Closed Lort533 closed 7 months ago

Lort533 commented 7 months ago

As in title. The code already seems to have enough amount of XSS prevention, but this PR prevents any non-number data to be displayed on the webpage. It's not a big deal on first glance, but RYD is 3rd party so it should be primary to treat it as "unsafe" (I mean yes, it's popular, but that's a good security measure to treat it as "unsafe" anyways), to even prevent unwanted text on the webpage in case of RYD getting hacked. Any non-number data will be displayed as NaN, as expected (since this should never happen if RYD works properly, otherwise there's something wrong that should be fixed).

As of now, anything that isn't a number can be displayed anyways, it's not a big deal since textContent seems to prevent XSS, but it would be scary for users to see a message like "You're hacked by XYZ".

lightbeam24 commented 7 months ago

Never thought of this, thanks. I just added it to my in-development version and it seems to work fine, so it'll be in the next beta (which is probably releasing ahead of schedule to include compatibility with YouTube's new watch grid layout).

Gonna be honest, this would be the first pull request I haven't rejected, so I'm actually kinda not sure what to do here. Not sure if I should merge this pull request, since there will not be any new updates yet, and the emerald.js in my development version has already changed a bit since Beta 1's release (not the part of the code that this alters, but still).

Lort533 commented 7 months ago

From what I think, if you merge this request and this code will also already be in your local Git project then I think it should merge just fine. If this code wasn't in your private Git project, it could make a merge conflict, but I really doubt in this situation.

That depends on your editor, afaik VCS has an option to synchronize commits to and from main GitHub project (locally iirc), but I don't know if that's the case here. I think the best way would be to backup the project before doing so.

lightbeam24 commented 7 months ago

I've definitely been doing GitHub wrong because I don't use Git at all. I do everything in Notepad++, and then use GitHub's upload function to upload the updated files once a new update is ready to release. I've been told this is the wrong way to do things, but changing my workflow could be a lot of work.

Lort533 commented 7 months ago

Then it should be fine merging it.

If you'd like to improve that workflow (with Visual Studio Code - yes, I prefer Notepad++ too though), let me know.

lightbeam24 commented 7 months ago

Very well. I will try it.