If you have an error on the checkout screen after clicking submit, or you have to go "Back" from the review order screen, the credit card fields are filled with values like "Last 4 digits (####)" and for CCV (----). But, you have to overwrite these values to proceed. You have to overwrite these values with the real values again. I think many users will expect these values to be stored someplace hidden so they don't have to enter them again -- the filled in values seem to suggest that the system knows the CC and CCV and is just not displaying the previously entered values. I'm suggesting these values get wiped on error or "Back" and just show empty fields so users will know they have to enter the data again.

CommentFileSizeAuthor
#5 396724.cc_fix.2.x.patch678 bytescha0s
#3 cclast4check.patch701 bytesadamo

Comments

rszrama’s picture

rszrama commented
Assigned: Unassigned » rszrama
Status: Active » Postponed (maintainer needs more info)

Hmm... the values actually should be getting stored, and this bug has come up in the past... so, I'm not entirely sure where to begin looking. Is this an issue you can duplicate on the 2.x Livetest?

chrisschaub’s picture

chrisschaub commented

Yes, the bug exists on the v2 livetest. The cc is filled with "(Last4)1111" which throws the error:

"You have entered an invalid credit card number."

So, to repeat:

1. Complete checkout screen, and continue with the "Review Order" button.
2. On review order screen, click the "Back" button.
3. From the checkout screen, click "Review Order" button.
4. You will see the error about the cc number being bad.

adamo’s picture

adamo commented
StatusFileSize
new cclast4check.patch701 bytes

I also have this issue. Issue still exists in livetest. I did a little debugging. The issue is that spaces are being stripped from the submitted credit card number before the bit of code that checks for "(Last 4) ". Changing the check to look for "(Last4)" solves the problem. Patch attached.

Island Usurper’s picture

Island Usurper commented
Status: Postponed (maintainer needs more info) » Needs review
cha0s’s picture

cha0s commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 396724.cc_fix.2.x.patch678 bytes

+1

Applied the patch on the livetest and it allowed me to checkout. I attached a patch that's a bit easier to apply, same code.

rszrama’s picture

rszrama commented
Status: Reviewed & tested by the community » Fixed

Confirmed the fix. Way to go guys! I think my problem with testing was using the admin account who could view the whole CC number all the time. : P

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

budda’s picture

budda
Primary language English
Location Cheshire
commented
Version: 6.x-2.0-beta5 » 5.x-1.7
Status: Closed (fixed) » Active

Can this be fixed in 5.x too please? It's such a small one...

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Confirming this also affects 5.x-1.7.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Reviewed & tested by the community
budda’s picture

budda
Primary language English
Location Cheshire
commented
Status: Reviewed & tested by the community » Active

The CVV and Issue number also have a similar problem with invalid values upon form re-submission.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented

IMO, we should not populate CC data fields (CC#, CV, EXP) when displaying the form. I don't know of any credible online sales sites that follow this practice.

Even if we are serving the data via SSL (I'd like to assume that, but ...), it seems foolhardy to serve a CC value back to the client who has just typed it in. Non-sensitive data, sure, but not CC data.

Doing so has limited benefits
* it makes it slightly easier for people to fill out the values
... and severe potential disadvantages ...
* browser might cache page contents incl CC details to disk (default behaviour in some versions of IE?)
* user may be alarmed to see their CC data appear onscreen when this is not the standard practice

My suggestion is that the value of the sensitive data fields on CC forms should be reset to blank whenever the page is sent back to the client.

I would be interested to hear of any (credible) example sites or online payment processors which follow the practice of populating CC data when a form error is detected. I don't know of any.

I appreciate the spirit of this bug report, and do agree that fixing it would make life slightly easier in a few cases, but I feel that we should seriously consider the security implications of such a bugfix before accepting it. I'm not here to rain on the parade, and happy to be told that this is a fine practice if that is indeed the case.

In the hope of engaging a bit more discussion of this, I've also started a similar thread on the UC forums:

http://www.ubercart.org/forum/development/10905/displaying_cc_data_when_...

Update: Ryan has replied to that post and stated that he has no intention of the CC details being displayed in that form.

budda’s picture

budda
Primary language English
Location Cheshire
commented

@xurizaemon I don't see anything here that suggests anybody would want the full ccard number to be displayed in the form. Try reading the issue before having a rant ;-)

tr’s picture

tr commented
Status: Active » Fixed

This was fixed for 1.x in #467490: Invalid credit card problem.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.