Juice-shop: "Export My Data" feature

[bkimminich]

_As a_ registered user of the Juice Shop
_I want_ to export all my personally identifiable information
_and_ my transaction data (like purchase history, reviews, feedback)
_so that_ my rights as promised by GDPR are fulfilled.

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[aaryan01]

Hey @bkimminich,
What should be the ideal export format? I think I'll be able to retrieve the data, export it in JSON format, convert it to excel (A few Node packages exist for that) and serve it to the user (but it won't have the profile picture if it is required at all). Thanks.

[bkimminich]

Hi! I think a JSON dump would be okay for the beginning. We could think about doing Excel as a selectable option at some point and then have a challenge where you sneak in dangerous macro into the Excel generator somehow... Probably we need new difficulty level :star::star::star::star::star::star::star: then... :grinning:

[aaryan01]

Hey @bkimminich @J12934
I'm a bit stuck at this
To start off, I created a component (a button) which on click would call a function (getUserData), which in turn would call the "whoAmI" function. Something like this:

The whoAmI function is always returning an empty object. I adopted the implementation from the user.service.ts file but I'm guessing that a token is missing?
I could really use some help.

[bkimminich]

Hm, just by looking at the screenshot I cannot see through the problem you ran into. The backend call happens? Did you try just using the existing function from the userService? Eventually you'll need your own backend endpoint as all data of the user should be collected and sent as JSON. No such backend endpoint exists currently.

[aaryan01]

@bkimminich I was working on this issue and I came across another bug (possibly) - I set up my username from the profile page but it is not reflecting in the user object const loggedInUser = insecurity.authenticatedUsers.get(req.cookies.token)
I'll open a new issue once you confirm this.

Also, I am exporting the user's profile data in a pdf. What all content should be there? Username, email, past orders. Am I missing something?

[bkimminich]

@aaryan01 While your PDF export in #881 certainly looks nice, I think in the final version with all user data (including feedback, reviews and recycling requests) it would get quite a lot of code for all those tables.

Recommendation

1. Make only a simple JSON import first. That could be possible even with a single complex joined query over all relevant DB tables via the UserId foreign keys.
2. Optionally make the PDF export but with the table-code being generic and you just pass in data which will be added on the PDF. Even the JSON from 1. could be used here to retrieve and pass in the data.
3. Later I think this would actually be a nicer to go for an extra Excel export and make it a challenge to include some _Formula Injection_ e.g. by submitting a feedback in the name of some user that contains such a payload and wait for that user to trigger their data export. See https://github.com/bkimminich/juice-shop/issues/660#issuecomment-462753919.

[aaryan01]

Hi @bkimminich ! Thanks for your review. I'll work on your suggestions :)

[agrawalarpit14]

Sorry for the reference above, I did it mistakenly.

[agrawalarpit14]

Hi @bkimminich Shall I implement data export in the .xslx(Excel) format? I was planning to employ json2xls for the same.
Thank You!!

[aaryan01]

export it in JSON format, convert it to excel (A few Node packages exist for that)

Hi @agrawalarpit14! I had planned on it but first @bkimminich wanted a basic JSON implementation. You can go ahead and work on it if you wish but I'd really like to complete it.
Thanks!

[agrawalarpit14]

Hi @agrawalarpit14! I had planned on it but first @bkimminich wanted a basic JSON implementation. You can go ahead and work on it if you wish but I'd really like to complete it.
Thanks!

Sure you can go on with it.✌🏻😁
I thought you weren't working on it, so I asked.😅
Thanks!!

[bkimminich]

@aaryan01 @agrawalarpit14, before doing any other formats lets make sure that

• [ ] API tests for the endpoint exist (including CAPTCHA testing)
• [ ] All the user data is in the JSON (including Recycles, Feedback, Complaints, Reviews etc.)

[aaryan01]

Hi @bkimminich , I'll start working on the 2nd point.
Thanks!

[agrawalarpit14]

@aaryan01 @agrawalarpit14, before doing any other formats lets make sure that

• [ ] API tests for the endpoint exist (including CAPTCHA testing)
• [ ] All the user data is in the JSON (including Recycles, Feedback, Complaints, Reviews etc.)

Sure I will write the tests.

[bkimminich]

Just make sure you don't step on each others toes, guys... :footprints:

[agrawalarpit14]

Just make sure you don't step on each others toes, guys... 👣

Shall I wait for the completion of the second point?
Or I will work only on order data as of now and then later add other tests.

[bkimminich]

Quality-wise it would be better to have the tests for the _current_ endpoint behavior done and then start _changing_ its behavior, so you'll notice mistakes by failing tests, but the other way around is also okay. In the end there should be API tests which cover every use case and error situation with the CAPTCHA as well, independent of the data output.

[agrawalarpit14]

Quality-wise it would be better to have the tests for the _current_ endpoint behavior done and then start _changing_ its behavior, so you'll notice mistakes by failing tests, but the other way around is also okay. In the end there should be API tests which cover every use case and error situation with the CAPTCHA as well, independent of the data output.

Sure. Then I will work on it right away and then extend it later when all the components are added.

[bkimminich]

Just make sure you don't step on each others toes, guys... footprints

What I meant is, that is probably isn't a good idea if one of you is writing the _current_ tests while the other is already _changing_ the endpoint, because then you'll have spent effort into testing stuff that maybe didn't even make it into the final outcome.

[agrawalarpit14]

What I meant is, that is probably isn't a go

Yeah true. I will be careful and won't spend time on testing other components which are not yet merged.
Thank You for the advise😇

[aaryan01]

Thank you for the suggestion @bkimminich :+1: ,
@agrawalarpit14 and I will keep each other and you posted at all steps :)
Thanks!

[aaryan01]

Hey @bkimminich @agrawalarpit14 , I'll start working on expanding the data export now.
Thanks!

[J12934]

The Data export feature is currently vulnerable to the similar kind of injection as the NoSQL Tier 3 challenge. If you register a user with a similar e-mail address like this [email protected]' || !0 || ' and export the data, you'll get the data exported from all users.

What gives it a little twist is that the export controller replaces all a|e|i|o|u with *, so that you can use keyword like true as they would turn into tr**.

Having to inject the payload into the e-mail is also interesting as the attacker has to register the user without the frontend to avoid the validation.

[agrawalarpit14]

The Data export feature is currently vulnerable to the similar kind of injection as the NoSQL Tier 3 challenge. If you register a user with a similar e-mail address like this [email protected]' || !0 || ' and export the data, you'll get the data exported from all users.

What gives it a little twist is that the export controller replaces all a|e|i|o|u with *, so that you can use keyword like true as they would turn into tr**.

Having to inject the payload into the e-mail is also interesting as the attacker has to register the user without the frontend to avoid the validation.

Hi @J12934 It sounds very interesting to me.
I think it would be awesome to make a challenge based on this.

But, in the comment about changing a|e|i|o|u to * . Do you mean that making this change in the email query(while queuing database) will make the attack little more difficult as the user would need to use queries such as [email protected]' || !0 || '.

If I understand you right and if the change(a|e|i|o|u to *) is done then the email query won't match anything in the database(as [email protected] will be compared to f**@b*r.c*m). and we will need to do the conversion over the email selectively.

Please let me know of your suggestions.
Thank You!!😁😁

[J12934]

The emails in the order are stored without their vowels, as some sort of pseudonymization i guess. The current implementation tries to find the ones with the same pseudonymization mails.

Thats done here:
https://github.com/bkimminich/juice-shop/blob/4b45706055ee79038575fb729579fc7f707445e1/routes/dataExport.js#L11-L17

The vulnerability isn't about the pseudonymization. The pseudonymization turns the injection payload from [email protected]' || !0 || ' to f**@b*r.c*m' || !0 || ' which still works.

[J12934]

Even without the injection you could make a challenge due to collisions with the pseudonymization. E.g you could "steel" the data export from [email protected] by registering a user with the email [email protected]. Both their mails would turn into b*nd*r@j*ic*-sh.*p and both would get the same export.

[bkimminich]

Even without the injection you could make a challenge due to collisions with the pseudonymization. E.g you could "steel" the data export from [email protected] by registering a user with the email [email protected]. Both their mails would turn into bndr@jic-sh.*p and both would get the same export.

This would be an amazing challenge... but we'd have to make sure this is the only vulnerability that works to steal someones export, otherwise everyone will choose an easier way...

[agrawalarpit14]

Even without the injection you could make a challenge due to collisions with the pseudonymization. E.g you could "steel" the data export from [email protected] by registering a user with the email [email protected]. Both their mails would turn into b*nd*r@j*ic*-sh.*p and both would get the same export.

Hi @J12934 @bkimminich Shall I implement the challenge in the said manner.
The challenge will be marked as solved only

• If the user follows such a technique [email protected] for [email protected] and successfully exports data, or
• If the user exports data for all users(containing [email protected]) by using a query such as [email protected]' || !0 || '. Although, I am not sure of this case as currently I am not able to login with such an email(I tried few variations, all of them get registered but I am unable to log in.)

Please let me know if the second point is needed and your views on this.
Thank You!!😆😇

[bkimminich]

The challenge shouldn't be to register the user that causes this clash, that should be the exploit with the challenge being to "Export someone else's data without using Injection" for example. Question is, can we check this in a way without baking another totally artificial challenge?

[agrawalarpit14]

The challenge shouldn't be to register the user that causes this clash, that should be the exploit with the challenge being to "Export someone else's data without using Injection" for example. Question is, can we check this in a way without baking another totally artificial challenge?

I am not sure I follow completely. Do you mean that we should find a way to solve the challenge "Export someone else's data without using Injection" in the case when there is no pseudonymization in the app and rather there is something more realistic?
Please let me know
Thank You!!🙂

[bkimminich]

No, exploiting the simplistic pseudonymization and then using normal export function should be the attack path, not just doing a NoSQL Injection. That's what I meant.

[bkimminich]

{ "username": "", "email": "[email protected]", "orders": [ { "orderId": "5267-08697927275a71c9", "totalPrice": 8.96, "products": [ { "quantity": 3, "name": "Apple Juice (1000ml)", "price": 1.99, "total": 5.97 }, { "quantity": 1, "name": "Orange Juice (1000ml)", "price": 2.99, "total": 2.99 } ], "eta": "3" }, { "orderId": "5267-2ac6c7c669d5fbdf", "totalPrice": 26.97, "products": [ { "quantity": 3, "name": "Eggfruit Juice (500ml)", "price": 8.99, "total": 26.97 } ], "eta": "2" } ] }

Everything is there already, only the challenge definition and check during export that someone elses data is in the JSON result is needed.

[agrawalarpit14]

I am very sorry, I got confused. I understand it now.
Shall I go about making it then?

[J12934]

@agrawalarpit14 please have a chat with @aaryan01 before you do.
He said in PR #938 that he planned to make some code changes to the data export logic.

[agrawalarpit14]

Sure, I will have a chat with him.
Thank You!!😁😁

[agrawalarpit14]

Hi @aaryan01 If the work on data export for reviews is complete, shall I add tests for it?

[aaryan01]

Hey! Sure go for it.

[agrawalarpit14]

Hi @aaryan01 Please let me know if you want me to work on customer feedbacks, recycle requests or complaints for data export, if you haven't started already?😁😁

[aaryan01]

@agrawalarpit14 Yes I do actually but I am in the middle of my end semester exams right now, I have started it and will complete it as soon as I am free.
Thanks :)

[agrawalarpit14]

Sure @aaryan01. All the best for your exams, I also finished just now.😁✌🏻

[bkimminich]

• [ ] 🔧 All export formats do JSON right now. In order close this ticket, we could just disable Excel/XML format radio buttons. Excel could be written later together with a Formula Injection challenge, maybe.

[lock[bot]]

This thread has been automatically locked because it has not had recent activity after it was closed. :lock: Please open a new issue for regressions or related bugs.