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

Update team page JavaScript to prevent XSS #10097

Closed
1 task
jimchamp opened this issue Nov 27, 2024 · 6 comments · Fixed by #10105
Closed
1 task

Update team page JavaScript to prevent XSS #10097

jimchamp opened this issue Nov 27, 2024 · 6 comments · Fixed by #10105
Assignees
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] Module: JavaScript Issues related to the JavaScript functionality. [managed] Priority: 2 Important, as time permits. [managed] Theme: Security Type: Bug Something isn't working. [managed]

Comments

@jimchamp
Copy link
Collaborator

Problem

The content of our team page is largely generated with JavaScript. When new elements are constructed for this page, text content is often added by setting the innerHTML property. This way of creating elements could leave us vulnerable to cross-site scripting attacks (more information here).

Reproducing the bug

  1. Go to ...
  2. Do ...
  • Expected behavior:
  • Actual behavior:

Context

  • Browser (Chrome, Safari, Firefox, etc): Any
  • OS (Windows, Mac, etc): All
  • Logged in (Y/N): N
  • Environment (prod, dev, local): prod

Breakdown

Since none of the text content used to generate the team member cards contains markup, it may be easiest to use textContent instead of innerHTML when creating the cards.

Requirements Checklist

  • In team.js, replace all instances of innerHTML with textContent

Related files

Stakeholders


Instructions for Contributors

  • Please run these commands to ensure your repository is up to date before creating a new branch to work on this issue and each time after pushing code to Github, because the pre-commit bot may add commits to your PRs upstream.
@jimchamp jimchamp added Type: Bug Something isn't working. [managed] Module: JavaScript Issues related to the JavaScript functionality. [managed] Good First Issue Easy issue. Good for newcomers. [managed] Priority: 2 Important, as time permits. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] Theme: Security labels Nov 27, 2024
@ronibhakta1
Copy link
Contributor

Hey @jimchamp will you assign me the issue i will make the required changes as you have mentioned and get the checks done :)

@github-actions github-actions bot added the Needs: Response Issues which require feedback from lead label Nov 29, 2024
@ronibhakta1
Copy link
Contributor

Hey @jimchamp will you assign me the issue i will make the required changes as you have mentioned and get the checks done :)

i'm waiting for your response !!1

@phoenix7815
Copy link

Hello @jimchamp, I’m new to open source and contributing to projects. I’d be glad to get started as soon as you assign me this issue. Thank you!

@prasad14070
Copy link

Hello @jimchamp , we had discussion on slack as well, I would like to contribute on this issue , it would be great if you assign this to me. I’ll ensure that the usage of textContent is correctly implemented and tested. Thank you!

@yrpatel24
Copy link

Hi @jimchamp! I am looking to contribute to this issue as well. If no one is currently working on it I can help!

@jimchamp
Copy link
Collaborator Author

jimchamp commented Dec 2, 2024

Go for it @ronibhakta1!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue Easy issue. Good for newcomers. [managed] Lead: @jimchamp Issues overseen by Jim (Front-end Lead, BookNotes) [managed] Module: JavaScript Issues related to the JavaScript functionality. [managed] Priority: 2 Important, as time permits. [managed] Theme: Security Type: Bug Something isn't working. [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants