Juice-shop: [⭐] storage of entire credit card numbers

[cnotin]

:star: Challenge idea

Description

I notice that the Cards API returns the full credit card number, while the UI only shows the last digits

Underlying vulnerabilities

• entire card storage -> PCI/DSS
• returning more info than what's displayed

Expected difficulty

| :heavy_check_mark: / :x: | Difficulty |
|:------------------------:|:-------------------------------------|
| :heavy_check_mark: | :star: |
| :heavy_check_mark: | :star::star: |
| :heavy_check_mark: | :star::star::star: |
| :grey_question: | :star::star::star::star: |
| :grey_question: | :star::star::star::star::star: |
| :grey_question: | :star::star::star::star::star::star: |

Possible attack flow

SQL injection allows to dump credit card numbers (or other ways to obtain the DB content?) that could be reused for a purchase by the attacker
I have no idea how to implement a challenge to show the fact that entire numbers are returned by the API while it's masked in the UI

[bkimminich]

First thing would be to let the API only return the masked card numbers, then we can remove the masking in the client instead. That should leave only SQLi to get the full numbers. Here we could make a challenge very similar to the one to retrieve the users table, just for the cards table.

[bkimminich]

Implementation hint: I checked /api/Cards endpoint implementation, and it's already doing some voodoo to make sure you only get records belonging to the currently logged in user in the routes/payment.js middleware. That might be a good place to also perform the masking of the cardNum property instead of doing it in the client.

[PranjalAgni]

So by masking we mean to "encrypt the cardNum value" ?

[bkimminich]

No, just return **********1234 to the client, so asterisks and the last digits, like the client shows it today already. Client could then just show the value from the API response, because it's already masked.

[PranjalAgni]

Got it, please assign this issue to me

[bkimminich]

Credit card protection when using API has been implemented. Not sure if this needs to become a SQLi challenge, though. It'd be almost the same as with the Users table where the password column is not returned by the API in general.

[cnotin]

Good news :)

Actually, do we really need to store the full credit card number? I did not play that much with the payment mechanism, but does it check the number some way?
If we really need or want to store full numbers, then it would be a PCI/DSS violation and we could perhaps think of a separate challenge (via SQLI or other way?)

[bkimminich]

Anyone still planning to make a SQLi challenge named _PCI DSS WTF_ (or something similar) around the unencrypted credit card storage? Would be very similar to the existing _User Credentials_ challenge, more or less a clone even.

We could also spice it up by storing a (too simple) hash of the full number plus the partial number with some (but not enough) digits *ed out, so that a brute force attack would be possible on the *ed out digits...

Both options would be fine in my opinion...

[cnotin]

I like your idea of weak hashing, unfortunately I cannot work on it for now sorry!

[bkimminich]

The hashing idea might not be the way to do it in reality, though, right? I suppose the best way would be to have an attempted PCI DSS compliant storage which is messed up to allow some brute force via the also stored partial card number with not enough asterisks.

As this is beyond just card number obfuscation, I'll close this ticket and will create one explicitly for that challenge.