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

highlight-words libray is giving Math.random vunerbility scan as high #950

Closed
1 task done
shajithomas32 opened this issue Jan 19, 2024 · 5 comments
Closed
1 task done

Comments

@shajithomas32
Copy link

material-react-table version

in all vesions

react & react-dom versions

in all vesions

Describe the bug and the steps to reproduce it

highlight-words libray is giving Math.random vunerbility scan as high

Minimal, Reproducible Example - (Optional, but Recommended)

highlight-words libray is giving Math.random vunerbility scan as high

Screenshots or Videos (Optional)

highlight-words libray is giving Math.random vunerbility scan as high

Do you intend to try to help solve this bug with your own PR?

No, because I do not know how

Terms

  • I understand that if my bug cannot be reliably reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@KevinVandy
Copy link
Owner

I just rechecked the source code for highlight-words and the only place where it uses Math.random is in a snippet like this

while (num--) {
    str += HEX[(Math.random() * 36) | 0];
  }

It uses this code simply to create a small uuid key for identification of chunks to tell them apart.

Feel free to share more info about this vuln, as if it was a real issue I'd take it seriously, but I don't see the severity at all right now.

MRT itself using Math.random to create random width skeleton loaders.

These use cases of Math.random() in the front-end are extremely inconsequential. Neither of these use cases are relying on it for storing ids in databases or anything like that.

@KevinVandy KevinVandy closed this as not planned Won't fix, can't repro, duplicate, stale Jan 19, 2024
@shajithomas32
Copy link
Author

shajithomas32 commented Jan 20, 2024

Thank you so much for the response, and I understood what you mentioned. We are using Material-React-Table, and this is the only high warning we got because highlight-words is used by Material-React-Table. No other libraries used by Material-React-Table gave any warning. That means hilights-words is using the JS native Math.random() which has been deemed insecure. They can replace those functions with the latest JS crypto stuff here as a recommendation. What you commented makes sense, but if they can replace Math.random(), it will make our scan clean. We mentioned it only because this is the only warning we got when using Material-React-Table and sometimes companies refuse to use it just because of the warning even though its not anything significant. If this can be fixed it will resolve the scan issue and less hassles for me because in the company scan its showing as high, so just asking a clean up request if possible. Material-react-table is excellent and i dont want to use another react table if company ask me to stop using it . See the attached screenshot.

You can do fortify scan

2024_01_19_20_33_54_Settings
.


2024_01_19_21_50_51_2

@KevinVandy
Copy link
Owner

I would support them fixing this too, and understand the paperwork problem for some orgs. I will update as soon as possible if they fix it. Either of us could PR.

However, for anyone reading this and wondering if you should be concerned about this, the answer is no. These random values are being used just in the front end for some rendering, and barely even need to be unique. Math.random is fine for such use cases. However as said above, to help anyone out with paperwork, we can still change this.

@shajithomas32
Copy link
Author

shajithomas32 commented Jan 20, 2024

"Thank you so much for understanding. These scans seem a bit perplexing; they just don't reflect the real logic. I am also new to React. Although this is not a matter to be fixed, I agree. However, for paperwork purposes, it would be great if it could be looked into. If it gets fixed in both React 17 and 18 versions, that would be fantastic. I'm new to React , and thank you so much to the developer of Material-React-Table for taking the time to assist me. It's a great product with excellent support. You are all good human beings; be blessed.".

"Again, this is only for paperwork. Also, the scan is clean for Material-React-Table; there's only a warning from the highlighted words when it scanned our bundled minified main."

I was using mrt 2.0.0-beta.15, so if fixing PLeae update a version that support react 17, I will be upgrading to react 18 in coming months so fix for latest mrt will be be geat too . Thanks again for considering fix for highlight word not directly involved with mrt

@shajithomas32
Copy link
Author

Sir , i did a pull request to fix it tricinel/highlight-words#35, can you apply it if are you contributor to hilight-words, i am new to this kind of process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants