-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
Fix Sensitive Data Exposure #2158
Fix Sensitive Data Exposure #2158
Conversation
#2852 Bundle Size — 10.25MiB (0%).9b42e71(current) vs 37f76c4 master#2849(baseline) Warning Bundle contains 3 duplicate packages – View duplicate packages Bundle metrics
|
Current #2852 |
Baseline #2849 |
|
---|---|---|
Initial JS | 5.55MiB |
5.55MiB |
Initial CSS | 304.68KiB |
304.68KiB |
Cache Invalidation | 0% |
0% |
Chunks | 51 |
51 |
Assets | 171 |
171 |
Modules | 1495 |
1495 |
Duplicate Modules | 21 |
21 |
Duplicate Code | 0.84% |
0.84% |
Packages | 124 |
124 |
Duplicate Packages | 3 |
3 |
Bundle size by type no changes
Current #2852 |
Baseline #2849 |
|
---|---|---|
JS | 7.34MiB |
7.34MiB |
IMG | 2.48MiB |
2.48MiB |
CSS | 321.47KiB |
321.47KiB |
Fonts | 93.55KiB |
93.55KiB |
Other | 17.62KiB |
17.62KiB |
HTML | 13.58KiB |
13.58KiB |
Bundle analysis report Branch bnematzadeh:gladys-sec-1 Project dashboard
Generated by RelativeCI Documentation Report issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR 🙏
I wrote 2 small comments on the unit test 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems you forgot to run prettier
I modified this test and it passed successfully
@bnematzadeh There is an issue with eslint in your file, let me know if you need help fixing it |
Prettier was run on this code before the commit, but I don’t know what the issue is. Can you help me? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2158 +/- ##
=======================================
Coverage 98.50% 98.50%
=======================================
Files 867 867
Lines 14243 14245 +2
=======================================
+ Hits 14030 14032 +2
Misses 213 213 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for this PR! :)
Description
While reviewing the code, I encountered two vulnerabilities: one is the lack of access to call the /v1/user endpoint, and the other is the disclosure of sensitive user information, such as their encrypted passwords, etc. In
user.controller.js
, there is a function defined asgetUsers
, and according to the following code inroutes.js
, we see that it is possible to call this endpoint through a user who is simply logged into the system:In
getUsers
, various parameters can be received through the query string. I came across an interesting parameter namedfields
, which allows us to set multiple values using commas in the query string and send them. The value offields
is stored in thequeryParams
variable within the get function and is included in the query. The problem here is that, unlike other application routes that remove the password and other sensitive fields before sending the response, there is no such code in this case to prevent this, and we can read sensitive user fields from the server.For the proof of concept, I wrote a security test. Before running it, ensure that the request is sent from a user who is not an admin.