pyscript icon indicating copy to clipboard operation
pyscript copied to clipboard

TODO project is vulnerable to XSS/HTML injection attacks

Open dmdhrumilmistry opened this issue 1 year ago • 7 comments

Checklist

  • [X] I added a descriptive title
  • [X] I searched for other issues and couldn't find a solution or duplication
  • [X] I already searched in Google and didn't find any good information or help

What happened?

TODO example project is vulnerable to XSS/HTML injection attacks.

Payload Used:

<button onclick='javascript:confirm("vulnerable");document.location.href="https://dmdhrumilmistry.github.io"'>click me</button>

Result:

XSS output image

Issue: Since most of the devs will be learning pyscript referring the examples, they'll be making applications which will be vulnerable to XSS/HTML injection. We would appreciate if pyscript team can also use secure coding techniques while creating examples; this will help devs to build secure applications.

What browsers are you seeing the problem on? (if applicable)

No response

Console info

No response

Additional Context

creating a cheatsheet might help devs to avoid such common issues in their applications

dmdhrumilmistry avatar Apr 24 '23 18:04 dmdhrumilmistry

@dmdhrumilmistry I'm afraid I am not sure I am following ... that's just how any Web page works?

<!doctype html>
<button onclick='javascript:confirm("vulnerable");document.location.href="https://dmdhrumilmistry.github.io"'>click me</button>

you are not using a py-click handler, you are showing any valid HTML, so what has this project in particular to do with your example? 🤔

WebReflection avatar Apr 24 '23 18:04 WebReflection

@dmdhrumilmistry wait a minute ... are you saying that if you add HTML as TODO item content that gets injected as HTML? Not sure that's just intentional / by-design for demo sake, but if that's the case we might avoid tasks as HTML, yes 👍

WebReflection avatar Apr 24 '23 18:04 WebReflection

@dmdhrumilmistry I've just tested locally and also checked the file part where we use innerText, not innerHTML to add the task ... this is what I see so we might need a step-by-step way to reproduce your issue or maybe it was fixed already (a month ago) and it's not live yet.

/cc @antocuni @fpliger

Screenshot from 2023-04-24 20-43-57

edit This is odd ... last change is from a year ago or something around that code https://github.com/pyscript/pyscript/commit/707474e835a315f4e94ac1dae3ebebc563f4988a

WebReflection avatar Apr 24 '23 18:04 WebReflection

using innerText would fix the issue! ohh okay. cool. When will it be deployed?

dmdhrumilmistry avatar Apr 25 '23 02:04 dmdhrumilmistry

@dmdhrumilmistry I am not sure when it's planned, but maybe @antocuni or @FabioRosado or @fpliger would know. They are at PyCon right so there might be some unintentional delay in answering this, the good thing is that current project doesn't break our expectations so please be patient, thank you!

WebReflection avatar Apr 25 '23 08:04 WebReflection

I may be wrong since I've been focused on the pyscript.com side of things but I'm assuming that the next release will happen when the work for workers is a bit more stable 🤔

FabioRosado avatar Apr 25 '23 08:04 FabioRosado

okay cool!

dmdhrumilmistry avatar Apr 25 '23 08:04 dmdhrumilmistry

Sorry I missed the thread.

Yes, next release should be happening within a month timeframe (the big task holding the release is indeed workers, as @FabioRosado mentioned).

@WebReflection @dmdhrumilmistry am I reading correctly that, given the comments above around innerText, we can close this issue?

fpliger avatar May 10 '23 20:05 fpliger

@fpliger the label waiting for release is there to signal this is not a bug (anymore) and will be closed once we release. If we close it before that there's no signal the bug is gone because there's no opened issue so we should just ignore these labelled issues and close them once we release.

WebReflection avatar May 11 '23 12:05 WebReflection

Sorry I missed the thread.

Yes, next release should be happening within a month timeframe (the big task holding the release is indeed workers, as @FabioRosado mentioned).

@WebReflection @dmdhrumilmistry am I reading correctly that, given the comments above around innerText, we can close this issue?

@fpliger, I'll prefer to close this once after releasing the fix. Else team might miss the issue.

dmdhrumilmistry avatar May 11 '23 17:05 dmdhrumilmistry

@dmdhrumilmistry 👍

@WebReflection Ok. This is different than the process we were adopting earlier (I probably missed the changes). I guess the rationale here is reflecting the state of the latest released version vs the state of main. We are due to sit down and put down a project/contribution workflow and guidelines anyway, so let's make sure we talk through this when we have that conversation

fpliger avatar May 11 '23 18:05 fpliger

@fpliger in all fairness it was my initiative and I've asked in the channel if that was OK but I guess I wasn't too loud there.

The (personal) idea was to meet users expectations, see:

I'll prefer to close this once after releasing the fix. Else team might miss the issue.

to which I agree specially because we don't have a fast release cycle so that it's easier to see an open bug exist (avoiding duplicates before we release) and the label makes it easy to filter by label and close whatever released, once released.

Happy to discuss this or formalize it once we have a workflow page to look at +1

WebReflection avatar May 12 '23 11:05 WebReflection

sorry for popping in randomly with an "unrelated" statement.

"YOU GUYS/GALS/UNDISCLOSED ROCK"

ps: and keep on keepin' on

Po-code avatar May 31 '23 20:05 Po-code

@Po-code TY so much for the kind words!! So much appreciated!! ❤️

fpliger avatar Jun 02 '23 15:06 fpliger

Well met friend! But It is i that thanks you. Im not much of a contributer to the open source (publicly anyways) as a result..my github profile hasnt earned badges and the repo isnt populated with any software/libraries/packages that I have built or hacked together lol.. Anyways if you want to know more about some of things I built or feel like maybe even incorporating some of them into some of your or your teams projects. Dont hesitate to let me know and ill gladly send you the works with hopes of feedback or some advice/pointers.

On Fri, Jun 2, 2023, 10:09 AM Fabio Pliger @.***> wrote:

@Po-code https://github.com/Po-code TY so much for the kind words!! So much appreciated!! ❤️

— Reply to this email directly, view it on GitHub https://github.com/pyscript/pyscript/issues/1405#issuecomment-1573891145, or unsubscribe https://github.com/notifications/unsubscribe-auth/AS2XQS3OIMLO7JUHC627ZITXJH64FANCNFSM6AAAAAAXJ5RVRQ . You are receiving this because you were mentioned.Message ID: @.***>

Po-code avatar Jun 05 '23 17:06 Po-code

I believe this got released by this time :sweat_smile:

WebReflection avatar Dec 06 '23 16:12 WebReflection

Hello again. Cant believe half a year has gone by since my previous comment. Anyways my statement still stands

p.s. still keep on rockin

Po-code avatar Dec 07 '23 12:12 Po-code

Hello again. Cant believe half a year has gone by since my previous comment.

apologies we've been busy "changing the world" but it's still our duty to go through the backlog and close "the closable", I hope you can forgive us for the wait.

Anyways my statement still stands

There is no XSS attack vector in current release and nothing in here is a vector for XSS so your statement should be reconsidered? If you can provide any XSS that shows something undesired and that's not already part of the HTML / W3C standard, we'd be happy to fix it ASAP too.

So far, that example was not presented or not valid with our current offer.

WebReflection avatar Dec 07 '23 13:12 WebReflection

Hello again. Cant believe half a year has gone by since my previous comment.

apologies we've been busy "changing the world" but it's still our duty to go through the backlog and close "the closable", I hope you can forgive us for the wait.

Anyways my statement still stands

There is no XSS attack vector in current release and nothing in here is a vector for XSS so your statement should be reconsidered? If you can provide any XSS that shows something undesired and that's not already part of the HTML / W3C standard, we'd be happy to fix it ASAP too.

So far, that example was not presented or not valid with our current offer.

im sorry let me clear this up for you, I never was apart of the XSS subject of this conversation I only left a comment that was unrelated to the topic, that is what I meant when I said "statement", as in, from my previous comment 6 months ago. I posted this as a renewal of my compliment for/to/of the py-script team/collab devs. And Why? because I got an email notification that that linked me to here. Hope this helps.

Po-code avatar Dec 07 '23 19:12 Po-code