Jump to content
peterbuzzin

PayPal App v5.018 Log In with PayPal is now dead

Recommended Posts

On 5/17/2019 at 2:56 PM, MrPhil said:

I'm still looking at the differences, and where the code will come from upon pressing the Update button. I'm also concerned about a future update (say, beyond 5.018) regressing to incompatible code (e.g., expecting $HTTP_POST_VARS, etc.). If it's code maintained by HPDL, that's a very real possibility. If it's from PayPal, it still might happen.

@MrPhil The paypal app code is hosted on oscommerce.com and most likely maintained by HPDL.  From the following URL  https://apps.oscommerce.com/index.php?Download&paypal&app&2_300&5_018&update for example will download the latest version in a zip file.  So unless HPDL updates the codebase to refect changes it will overwrite when pressing the auto-update button.  But it's easy enough to change the auto-update URL so it points to a different repository that contains non-breaking/compatible archives which will then effectively cut off HPDL updates but as long as you mirror any updates with code amended for Frozen.  (I'm considering doing the same as I've customised the PP modules heavily and if one of my clients hit the update button it would be lost so I've hidden it for now)

On 5/17/2019 at 2:56 PM, MrPhil said:

"best not to use naked $_POST/$_GET"  could you offer some justification for that? It's been generally agreed upon that maintaining the old HTTP_ arrays is a bad thing, and one of the things @burt has been doing in the "BS" versions is changing all of them to the modern $_POST and $_GET forms. It would be stunning to discover that was a bad move!

What were the reasons for removing them, why is it a bad thing?  Seems like a lot of effort to remove something and replace it with something which is basically the same thing.  There's nothing bad about replacing them other than you'll lose the auto-escaping feature, and any existing modules that would have been compatible would need to be updated (swapping out $HTTP_POST_VARS for $_POST for example) for the sake of continuity.  Time could be better spent elsewhere IMO. 


If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites

I'll look at the PayPal app update some more. I just want to be very careful not to break things! My end goal is still to have Frozen 5.018 out of the box. It's just that Gary's version baked into Frozen is 1) apparently 4.039, and 2) has been updated for various CE-related global changes. It looks like Harald's PayPal app was updated to 5.010, but lacks compatibility with Gary's CE changes. I'm hoping that maybe Gary will adopt the Frozen patch for PayPal 5.018 into Edge.

1 hour ago, peterbuzzin said:

What were the reasons for removing them, why is it a bad thing?

Well, originally PHP had $HTTP_POST_VARS and $HTTP_GET_VARS arrays. These were removed (or at least, deprecated) a long time ago, replaced by $_POST, $_GET, and $_REQUEST. osC kept copies of them (the long names), mostly to avoid the work of changing them, until Gary decided to bite the bullet and change them in his CE work. I think the scoping rules are a little different between the old and new array versions. I seem to recall that the argument was it was better to get in line with the new way of doing things, and fix old add-ons, than to continue to muddle along with the old way of doing things and confusing people.


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites

It appears that there are incremental updates past 5.010. Do I have it right that I manually bring Frozen up to 5.010 with the PayPal app, and then apply the incremental updates 5.011,  14, 16, and 18 to be fully 5.018 ready? And then keep an eye open for further 5.xxx updates to come? I guess that HPDL is somehow keeping up to date with PayPal patches.


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites

Unfortunately I think HPDL isn't keeping up-to-date with PayPal patches.  The changes and feature deprecations at PayPal have been publicised for sometime now.

With regards to Frozen, I'm not aware of the point when it became a fork.  osCommerce v2.3.4.1 as downloaded from the homepage doesn't come with the PayPal App.  It has PayPal modules which are active at the time of installation but not the App.  The App adds additional includes/apps and includes/hooks directories, if you have these present then you're halfway there.

Hop you don't mind me asking, I'm happy to continue to help/advise but I'm just thinking this thread could go very off topic and I don't want the fixes to get lost amongst it all.  Would you mind creating a new thread and tagging me in it and we can carry on chatting about it there if you like.


If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites

I know I said about creating a separate thread (and I hope you still do) but until then I've just thought of something that could be a vulnerability with regards to Frozen and the removal of  $HTTP_POST_VARS and $HTTP_GET_VARS, what is being done to escape the $_POST and $_GET variables instead?

Without escaping them someone could easily perform Cross-Site Scripting (XSS) client side attacks/injections on form fields.  The $HTTP_POST_VARS and $HTTP_GET_VARS were a creation of do_magic_quotes_gpc() function in compatibility.php and even if they referred to the now deprecated PHP variables names offered basic protection against XSS.  Is there compensation for this by the use of a similar function to loop through all $_POST and $_GET arrays in frozen before they're used?  If not, then on forms where the original input is outputted back on the page (as an example) on submission if a naked echo $_GET['keyname'] is being used instead of $HTTP_GET_VAR['keyname'] this could/will have disastrous outcomes!

As an example, if you had <textarea><?php echo $_GET['keyname'];?></textarea> that could easily be turned into and output like....

<textarea>{start point of injection}</textarea> <script>naughty javascript inserted here</script>Enter your Card details:<input type="text" required></input> <textarea>{end point of injection}</textarea>


If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites
21 minutes ago, peterbuzzin said:

if a naked echo $_GET['keyname'] is being used instead of $HTTP_GET_VAR['keyname'] this could/will have disastrous outcomes!

could you not use htmlspecialchars($_GET['keyname'] )  


No longer giving free advice. Please place deposit in meter slot provided.  Individual: [=] SME: [==] Corporation: [===]
If deposit does not fit one of the slots provided then you are asking too much! :P


Support The Project
Documentation/Knowledgebase/Discussions
 

Share this post


Link to post
Share on other sites
1 hour ago, 241 said:

could you not use htmlspecialchars($_GET['keyname'] )  

You could, but you'd need to do that on every instance of echo $_POST or echo $_GET.  This was partly why I couldn't understand why the decision was made to change $HTTP_POST_VARS/$HTTP_GET_VARS to $_POST/$_GET (unless previously global functionality has been compensated for also [and that's global in terms of applying the process before utilisation of variables and not super global]) aside from a bit of a waste of effort.

Anyway, as mentioned this might be better in a separate thread/topic so this one can stay on topic.


If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites
On 5/21/2019 at 6:21 AM, peterbuzzin said:

I know I said about creating a separate thread (and I hope you still do)

At this point, I see no reason to carry this further, here. If you think there are security problems, feel free to start a new topic/thread and continue that part of the discussion there.

On 5/21/2019 at 6:21 AM, peterbuzzin said:

Without escaping them someone could easily perform Cross-Site Scripting (XSS) client side attacks/injections on form fields.  The $HTTP_POST_VARS and $HTTP_GET_VARS were a creation of do_magic_quotes_gpc() function in compatibility.php and even if they referred to the now deprecated PHP variables names offered basic protection against XSS.  Is there compensation for this by the use of a similar function to loop through all $_POST and $_GET arrays in frozen before they're used?

Perhaps someone more familiar with the work and reasons for changing to the short forms ( @burt ?) could speak to this. Has Pete found an area of legitimate concern, or is he mistaken? In places where these superglobals could potentially be used to inject nasties, I was under the impression that cleanup was done on a case-by-case basis rather than globally. Of course, this does increase the chance that some case will be overlooked! Are "magic quotes" still around? I thought I heard about their being withdrawn.

Certainly, we should always be on the lookout for places where $_* could be used to inject malicious code. Should cleanup be restricted to places where it could actually be used to do something bad (in HTML sent back to the browser, in database fields, etc.)? Is there such a thing as a universal cleanup that could be done?


If you are running the "official" osC 2.3.4 or 2.3.4.1 download, your installation is obsolete! Get (stable) Frozenpatches or (unstable) Edge. See also the naming convention and the latest community-supported responsive "Edge" release

Share this post


Link to post
Share on other sites
21 hours ago, MrPhil said:

cleanup was done on a case-by-case basis rather than globally

Correct. Nothing wrong with using directly _GET and _POST, so long as they are sanitized appropriately (tep_db_input, tep_db_prepare_input, tep_output_string etc).


This is a signature that appears on all my posts.  
IF YOU MAKE A POST REQUESTING HELP...please state the exact version
of osCommerce that you are using. THANKS

 
Get the latest Responsive osCommerce CE (community edition) here

Share this post


Link to post
Share on other sites

So not having "Connect with PayPal" Customers phone number is becoming problematic... Any suggestions or ideas on how a phone number can be requested (when missing) during checkout?

Thanks!


-Dave

Share this post


Link to post
Share on other sites
52 minutes ago, Roaddoctor said:

So not having "Connect with PayPal" Customers phone number is becoming problematic... Any suggestions or ideas on how a phone number can be requested (when missing) during checkout?

Thanks!

It's not possible to get a phone number from PayPal any longer.  Only option would be to ask the customer to enter the number after login.  You'd need to code a secondary page after login with PayPal that just has one field asking for the telephone number and if you wanted to make it compulsory, code it so that they cannot proceed past that page without entering a number.


If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites

is the original code in includes/modules/content/login/cm_paypal_login.php lines 212-220 correct

              if (ACCOUNT_STATE == 'true') {
                if ($ship_zone_id > 0) {
                  $sql_data_array['entry_zone_id'] = $ship_zone_id;
                  $sql_data_array['entry_state'] = '';
                } else {
                  $sql_data_array['entry_zone_id'] = '0';
                  $sql_data_array['entry_state'] = $ship_zone;
                }
              }

more so line 215

                  $sql_data_array['entry_state'] = '';

if you have the ship_zone_id which is derived from the query based on PayPal response for country and zone being from region should the line not be

                  $sql_data_array['entry_state'] = $ship_zone;

and line 218 be

                  $sql_data_array['entry_state'] = '';

 


No longer giving free advice. Please place deposit in meter slot provided.  Individual: [=] SME: [==] Corporation: [===]
If deposit does not fit one of the slots provided then you are asking too much! :P


Support The Project
Documentation/Knowledgebase/Discussions
 

Share this post


Link to post
Share on other sites
Posted (edited)

osC initialises the variable with $ship_zone_id = 0 on first processing.

It then queries the DB to find the country provided by PayPal in the shipping address contained in PayPal

If the country is present in osC it then queries again to see if the region (contained in $ship_zone) provided by PayPal exists in the osC DB as a zone name (Alabama for example) or zone code (AL), if it does then $ship_zone_id is given the ID (1) contained in osC DB

So later on when it gets to approx 213 it looks to see if $ship_zone_id is greater than zero (the result of whether a matching region was returned).

If it's greater than zero then it's applied to $sql_data_array['entry_zone_id']

If it's not then instead of storing the ID against entry_zone_id (because it doesn't exist) it saves the name/text of the region provided by PayPal in entry_state.

It's the exact same process/logic as create_account.php, it doesn't save the text value of the state/region because the state ID is already present in table zones.

 

Edited by peterbuzzin

If it still don't work, hit it again!

Senior PHP Dev with 18+ years of experience for hire, all requirements considered, see profile for more information.

Is your version of osC up to date? You'll find the latest osC version (the community-supported responsive version) here.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×