[freeside-commits] branch master updated. 85fead435919494470c22f7785a78a4516593a9d
Mitch Jackson
mitch at freeside.biz
Sun Aug 26 15:22:25 PDT 2018
The branch, master has been updated
via 85fead435919494470c22f7785a78a4516593a9d (commit)
via c8db17c0273a35c696ca9d70f3462f868f7873f8 (commit)
via 7a177f82917a57bc1a0a8ef4a4608aca24773624 (commit)
from 6edcac3fb4f343273195460f03a5e3c01feecb2a (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 85fead435919494470c22f7785a78a4516593a9d
Author: Mitch Jackson <mitch at freeside.biz>
Date: Sun Aug 26 18:11:15 2018 -0400
RT# 80869 freeside_upgrade fix for bad payment expiration dates
diff --git a/FS/FS/cust_payby.pm b/FS/FS/cust_payby.pm
index 477700dc8..6d7ece6ca 100644
--- a/FS/FS/cust_payby.pm
+++ b/FS/FS/cust_payby.pm
@@ -921,9 +921,89 @@ sub _upgrade_data {
local $ignore_expired_card = 1;
local $ignore_invalid_card = 1;
$class->upgrade_set_cardtype;
+ $class->_upgrade_data_paydate_edgebug;
}
+=item _upgrade_data_paydate_edgebug
+
+Correct bad data injected into payment expire date column by Edge browser bug
+
+The month and year values may have an extra character injected into form POST
+data by Edge browser. It was possible for some bad month values to slip
+past data validation.
+
+If the stored value was out of range, it was causing payments screen to crash.
+We can detect and fix this by dropping the second digit.
+
+If the stored value is is 11 or 12, it's possible the user inputted a 1. In
+this case, the payment method will fail to authorize, but the record will
+not cause crashdumps for being out of range.
+
+In short, check for any expiration month > 12, and drop the extra digit
+
+=cut
+
+sub _upgrade_data_paydate_edgebug {
+ my $journal_label = 'cust_payby_paydate_edgebug';
+ return if FS::upgrade_journal->is_done( $journal_label );
+
+ my $oldAutoCommit = $FS::UID::AutoCommit;
+ local $FS::UID::AutoCommit = 0;
+
+ for my $row (
+ FS::Record::qsearch(
+ cust_payby => { paydate => { op => '!=', value => '' }}
+ )
+ ) {
+ next unless $row->ut_daten('paydate');
+
+ # paydate column stored in database has failed date validation
+ my $bad_paydate = $row->paydate;
+
+ my @date = split /[\-\/]/, $bad_paydate;
+ @date = @date[2,0,1] if $date[2] > 1900;
+
+ # Only autocorrecting when month > 12 - notify operator
+ unless ( $date[1] > 12 ) {
+ die sprintf(
+ 'Unable to correct bad paydate stored in cust_payby row '.
+ 'custpaybynum(%s) custnum(%s) paydate(%s)',
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ );
+ }
+
+ $date[1] = substr( $date[1], 0, 1 );
+ $row->paydate( join('-', @date ));
+
+ if ( my $error = $row->replace ) {
+ die sprintf(
+ 'Failed to autocorrect bad paydate stored in cust_payby row '.
+ 'custpaybynum(%s) custnum(%s) paydate(%s) - error: %s',
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ $error
+ );
+ }
+
+ warn sprintf(
+ 'Autocorrected bad paydate stored in cust_payby row '.
+ "custpaybynum(%s) custnum(%s) old-paydate(%s) new-paydate(%s)\n",
+ $row->custpaybynum,
+ $row->custnum,
+ $bad_paydate,
+ $row->paydate,
+ );
+
+ }
+
+ FS::upgrade_journal->set_done( $journal_label );
+ dbh->commit unless $oldAutoCommit;
+}
+
=head1 BUGS
=head1 SEE ALSO
commit c8db17c0273a35c696ca9d70f3462f868f7873f8
Author: Mitch Jackson <mitch at mitchjacksontech.com>
Date: Sun Aug 26 17:11:38 2018 -0400
RT# 80869 Improve cust_payby.paydate validation
diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm
index cf8ec4d73..e951178cc 100644
--- a/FS/FS/Record.pm
+++ b/FS/FS/Record.pm
@@ -3219,6 +3219,60 @@ sub ut_enumn {
: '';
}
+=item ut_date COLUMN
+
+Check/untaint a column containing a date string.
+
+Date will be normalized to YYYY-MM-DD format
+
+=cut
+
+sub ut_date {
+ my ( $self, $field ) = @_;
+ my $value = $self->getfield( $field );
+
+ my @date = split /[\-\/]/, $value;
+ if ( scalar(@date) == 3 ) {
+ @date = @date[2,0,1] if $date[2] >= 1900;
+
+ local $@;
+ my $ymd;
+ eval {
+ # DateTime will die given invalid date
+ $ymd = DateTime->new(
+ year => $date[0],
+ month => $date[1],
+ day => $date[2],
+ )->ymd('-');
+ };
+
+ unless( $@ ) {
+ $self->setfield( $field, $ymd ) unless $value eq $ymd;
+ return '';
+ }
+
+ }
+ return "Illegal (date) field $field: $value";
+}
+
+=item ut_daten COLUMN
+
+Check/untaint a column containing a date string.
+
+Column may be null.
+
+Date will be normalized to YYYY-MM-DD format
+
+=cut
+
+sub ut_daten {
+ my ( $self, $field ) = @_;
+
+ $self->getfield( $field ) =~ /^()$/
+ ? $self->setfield( $field, '' )
+ : $self->ut_date( $field );
+}
+
=item ut_flag COLUMN
Check/untaint a column if it contains either an empty string or 'Y'. This
diff --git a/FS/FS/cust_payby.pm b/FS/FS/cust_payby.pm
index 704741f3d..477700dc8 100644
--- a/FS/FS/cust_payby.pm
+++ b/FS/FS/cust_payby.pm
@@ -315,7 +315,7 @@ sub check {
#encrypted #|| $self->ut_textn('payinfo')
#encrypted #|| $self->ut_textn('paycvv')
# || $self->ut_textn('paymask') #XXX something
- #later #|| $self->ut_textn('paydate')
+ || $self->ut_daten('paydate')
|| $self->ut_numbern('paystart_month')
|| $self->ut_numbern('paystart_year')
|| $self->ut_numbern('payissue')
commit 7a177f82917a57bc1a0a8ef4a4608aca24773624
Author: Mitch Jackson <mitch at freeside.biz>
Date: Sun Aug 26 17:07:46 2018 -0400
RT# 80869 Harden process payment screen against Edge browser bug
diff --git a/httemplate/elements/city.html b/httemplate/elements/city.html
index 4e9a60940..05250fef5 100644
--- a/httemplate/elements/city.html
+++ b/httemplate/elements/city.html
@@ -132,14 +132,14 @@ function <% $pre %>county_changed(what, callback) {}
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE="" <% $opt{city} eq '' ? 'SELECTED' : '' %>><% $opt{empty_label} %>
+ <OPTION VALUE="" <% $opt{city} eq '' ? 'SELECTED' : '' %>><% $opt{empty_label} %></OPTION>
% }
% foreach my $city ( @cities ) {
<OPTION VALUE="<% $city |h %>"
<% $city eq $opt{city} ? 'SELECTED' : '' %>
- ><% $city eq $opt{empty_data_value} ? $opt{empty_data_label} : $city %>
+ ><% $city eq $opt{empty_data_value} ? $opt{empty_data_label} : $city %></OPTION>
% }
diff --git a/httemplate/elements/cust_payby_new.html b/httemplate/elements/cust_payby_new.html
index 7ed049686..8b1d93d59 100644
--- a/httemplate/elements/cust_payby_new.html
+++ b/httemplate/elements/cust_payby_new.html
@@ -4,7 +4,6 @@
% my( $payinfo, $paycvv, $month, $year ) = ( '', '', '', '' );
% my $payname = $cust_main->first. ' '. $cust_main->getfield('last');
% my $location = $cust_main->bill_location;
-
<TR>
<TH ALIGN="right"><% mt('Card number') |h %></TH>
<TD COLSPAN=7>
@@ -15,21 +14,17 @@
<TH><% mt('Exp.') |h %></TH>
<TD>
<SELECT NAME="month">
-% for ( ( map "0$_", 1 .. 9 ), 10 .. 12 ) {
-
- <OPTION<% $_ == $month ? ' SELECTED' : '' %>><% $_ %>
+% for my $mm ( map{ sprintf( '%02d', $_ ) } (1..12) ) {
+ <OPTION value="<% $mm %>"<% $mm == $month ? ' SELECTED' : '' %>><% $mm %></OPTION>
% }
-
</SELECT>
</TD>
<TD> / </TD>
<TD>
<SELECT NAME="year">
-% my @a = localtime; for ( $a[5]+1900 .. $a[5]+1915 ) {
-
- <OPTION<% $_ == $year ? ' SELECTED' : '' %>><% $_ %>
+% my @a = localtime; for my $yyyy ( $a[5]+1900 .. $a[5]+1915 ) {
+ <OPTION value="<% $yyyy %>"<% $yyyy == $year ? ' SELECTED' : '' %>><% $yyyy %></OPTION>
% }
-
</SELECT>
</TD>
</TR>
@@ -162,7 +157,7 @@
<% mt('as') |h %>
<SELECT NAME="weight">
% for ( 1 .. 1+scalar(grep { $_->payby =~ /^(CARD|CHEK)$/ } @cust_payby) ) {
- <OPTION VALUE="<%$_%>"><% mt( $weight{$_} ) |h %>
+ <OPTION VALUE="<%$_%>"><% mt( $weight{$_} ) |h %></OPTION>
% }
</SELECT>
% } else {
diff --git a/httemplate/elements/select-country.html b/httemplate/elements/select-country.html
index c98147907..286826752 100644
--- a/httemplate/elements/select-country.html
+++ b/httemplate/elements/select-country.html
@@ -91,15 +91,13 @@ Example:
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE=""><% $opt{'empty_label'} || '(all)' %>
+ <OPTION VALUE=""><% $opt{'empty_label'} || '(all)' %></OPTION>
% }
% foreach my $country ( @all_countries ) {
-
- <OPTION VALUE="<% $country |h %>"
- <% $country eq $opt{'country'} ? ' SELECTED' : '' %>
- ><% FS::geocode_Mixin->code2country($country). " ($country)" %>
-
+ <OPTION VALUE="<% $country |h %>"<% $country eq $opt{'country'} ? ' SELECTED' : '' %>>
+ <% FS::geocode_Mixin->code2country($country). " ($country)" |h %>
+ </OPTION>
% }
</SELECT>
diff --git a/httemplate/elements/select-month_year.html b/httemplate/elements/select-month_year.html
index 62c10b15f..406c13b21 100644
--- a/httemplate/elements/select-month_year.html
+++ b/httemplate/elements/select-month_year.html
@@ -3,16 +3,15 @@
<% $empty ? '<OPTION VALUE="">' : '' %>
% foreach ( 1 .. 12 ) {
- <OPTION<% $_ == $mon ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $mon[$_-1] %>
+ <OPTION<% $_ == $mon ? ' SELECTED' : '' %> VALUE="<% sprintf('%02d', $_) %>"><% $mon[$_-1] %></OPTION>
% }
-
</SELECT>/<SELECT NAME="<% $prefix %>_year" SIZE="1" <% $disabled%>>
<% $empty ? '<OPTION VALUE="">' : '' %>
% for ( $start_year .. $end_year ) {
- <OPTION<% $_ == $year ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $_ %>
+ <OPTION<% $_ == $year ? ' SELECTED' : '' %> VALUE="<% $_ %>"><% $_ %></OPTION>
% }
</SELECT>
diff --git a/httemplate/elements/select-state.html b/httemplate/elements/select-state.html
index 3fb559734..8db157b92 100644
--- a/httemplate/elements/select-state.html
+++ b/httemplate/elements/select-state.html
@@ -27,16 +27,13 @@ Example:
>
% unless ( $opt{'disable_empty'} ) {
- <OPTION VALUE=""<% $opt{state} eq '' ? ' SELECTED' : '' %>><% $opt{empty_label} %>
+ <OPTION VALUE=""<% $opt{state} eq '' ? ' SELECTED' : '' %>><% $opt{empty_label} %></OPTION>
% }
% foreach my $state ( keys %states ) {
-
- <OPTION VALUE="<% $state |h %>"<% $state eq $opt{'state'} ? ' SELECTED' : '' %>><% $states{$state} || '(n/a)' |h %>
-
+ <OPTION VALUE="<% $state |h %>"<% $state eq $opt{'state'} ? ' SELECTED' : '' %>><% $states{$state} || '(n/a)' |h %></OPTION>
% }
-
</SELECT>
<%init>
diff --git a/httemplate/elements/select-table.html b/httemplate/elements/select-table.html
index a52fdfaaa..d86b7ee43 100644
--- a/httemplate/elements/select-table.html
+++ b/httemplate/elements/select-table.html
@@ -83,11 +83,11 @@ Example:
% || ( $value eq $pre_opt );
<OPTION VALUE="<% $pre_opt %>"
<% $selected ? 'SELECTED' : '' %>
- ><% $pre_label %>
+ ><% $pre_label %></OPTION>
% }
% unless ( $opt{'multiple'} || $opt{'disable_empty'} ) {
- <OPTION VALUE=""><% $opt{'empty_label'} || 'all' %>
+ <OPTION VALUE=""><% $opt{'empty_label'} || 'all' %></OPTION>
% }
% foreach my $record (
@@ -118,7 +118,7 @@ Example:
? &{ $opt{'label_callback'} }( $record )
: $record->$name_col()
|h
- %>
+ %></OPTION>
% }
% while ( @post_options ) {
@@ -128,7 +128,7 @@ Example:
% || ( $value eq $post_opt );
<OPTION VALUE="<% $post_opt %>"
<% $selected ? 'SELECTED' : '' %>
- ><% $post_label %>
+ ><% $post_label %></OPTION>
% }
</SELECT>
diff --git a/httemplate/misc/process/payment.cgi b/httemplate/misc/process/payment.cgi
index d0e589661..7747bcbea 100644
--- a/httemplate/misc/process/payment.cgi
+++ b/httemplate/misc/process/payment.cgi
@@ -100,11 +100,11 @@ if ( (my $custpaybynum = scalar($cgi->param('custpaybynum'))) > 0 ) {
# use new info
##
- $cgi->param('year') =~ /^(\d+)$/
+ $cgi->param('year') =~ /^(\d{4})/
or errorpage("illegal year ". $cgi->param('year'));
$year = $1;
- $cgi->param('month') =~ /^(\d+)$/
+ $cgi->param('month') =~ /^(\d{2})/
or errorpage("illegal month ". $cgi->param('month'));
$month = $1;
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Record.pm | 54 ++++++++++++++++++++
FS/FS/cust_payby.pm | 82 +++++++++++++++++++++++++++++-
httemplate/elements/city.html | 4 +-
httemplate/elements/cust_payby_new.html | 15 ++----
httemplate/elements/select-country.html | 10 ++--
httemplate/elements/select-month_year.html | 5 +-
httemplate/elements/select-state.html | 7 +--
httemplate/elements/select-table.html | 8 +--
httemplate/misc/process/payment.cgi | 4 +-
9 files changed, 156 insertions(+), 33 deletions(-)
More information about the freeside-commits
mailing list