18

Since Let's make this fun(-ish) turned out to be a real success and after a discussion from last week on Twitter, WE decided (there are more of us) that was a nice idea to continue "the fun" with funny / useless / horrible code from third party magento extensions.
Since nobody else wanted to create the thread (myself included), the screenshot I saw today made me do it.

All code you provide must be verifiable. It should come with a link to the extension repository or at least an extension name.

But be careful of what you post. Read the module licence first and see if you are allowed to post it.

GO!

16

OK. I'll get the ball rolling.
The following code is the result of an investigation I "conducted" for this question: Hidden values in an extension - encode decode data transaction.

You can find the extension here. Somewhere in the config.xml of the extension is this:

<sdatacode>
    ZXZhbChnemluZmxhdGUoYmFzZTY0X2RlY29kZSgnalpBeGI4TWdFSVhuK0ZmUUNCbDdxTDFVSFJMUkRGR2tMbFdqT00wU1ZSSEJWNE5xd0lLTHFxcktmeTg0UzlNdVhlRGdjZDk3QnpXaUk1eFFWRHJjUG5TQVQ2SURpMjRIUG1obmkzS2VVZVBhcXlmUDYrVlBXVjJKeTVOdjBIbDQ4WDFVSjVPTWhxaFBvdzMvb25FOTV4R1hTdGVlYzJXRmdYaFE1MXhJakVBdUZjajNhYVMyQWtWc1BJb0E5M2NIc05LMVVOQXdHaVppY1FObXdNK0NIcHJWWnJmYTdObmpkcnR1Mkd0Sjhwejh1ZVdjTTJkWnVXQUtjUWhzTnU0czBrNitUd09vVUxGWlhXdUw0TFh6SDNCc0llak9WdEtaMmx3K3BYYUQ3TFVFR3lDVmw4alZvSVpGU0hFNXE4Ylk4MHkvRldsQUlKd1RoaENRbGIrTmVpZEZyMXpBLzBLL0FRPT0nKSkpOw==
</sdatacode>

The value is later used for...

eval(gzinflate(base64_decode('jZAxb8MgEIXn+FfQCBl7qL1UHRLRDFGkLlWjOM0SVRHBV4NqwIKLqqrKfy84S9MuXeDgcd97BzWiI5xQVDrcPnSAT6IDi24HPmhni3KeUePaqyfP6+VPWV2Jy5Nv0Hl48X1UJ5OMhqhPow3/onE95xGXSteec2WFgXhQ51xIjEAuFcj3aaS2AkVsPIoA93cHsNK1UNAwGiZicQNmwM+CHprVZrfa7Nnjdrtu2GtJ8pz8ueWcM2dZuWAKcQhsNu4s0k6+TwOoULFZXWuL4LXzH3BsIejOVtKZ2lw+pXaD7LUEGyCVl8jVoIZFSHE5q8bY80y/FWlAIJwThhCQlb+NeidFr1zA/0K/AQ==')));

Yep...there's an eval there..,The evaluation translates into:

$mag = $this->getMagentoVersion(); 
$mod = $this->getOPCVersion(); 
$h = $this->getCurStoreUrl(); 
$s = "mag={$mag}&mod={$mod}&hname={$h}&action=check"; 
$data = base64_encode($s); 
$hs = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS']==='on')?'https':'http'; 
$url = $hs.'://interiorwebdesign.com/magento/opclicense/opcaction.php?sdata='.$data; 
if($mode == 'test') 
    $url = $hs.'://localhost/opclicense/opcaction.php?sdata='.$data;

... you guessed it. It's a way of phoning home.

  • 6
    If I'm on a secure page it securely phones home, but otherwise uses plain-text traffic. LOL! Sadly it seems all (most) of the major/reputable module vendors are doing this. Little Snitch running on an OS X dev box is good at ratting on them. – davidalger Jun 13 '14 at 17:48
  • 2
    @davidalger It's a bit worse. It doesn't handle SSL_OFFLOADED. Also, easy to hack around by setting $mode to test before and back to it's previous value after the eval. – Melvyn Jun 15 '14 at 20:49
  • 2
    @davidalger Not all I must to say :) – Amasty Jul 29 '14 at 22:43
  • 4
    @Amasty Alas, but you are still connecting to amasty.com on every admin pre-dispatch request to update your feed. Just as bad. :) – davidalger Jul 29 '14 at 22:46
  • 1
    @davidalger toucher! :) I used to think it's one request per admin session, the same as magento itself does, as there is cache lifetime. – Amasty Jul 29 '14 at 22:55
9

Handling credit card details when you shouldn't and who needs SSL anyways:

        if ($payment && $payment->getMethod() == Mage::getModel('authorizenet/directpost')->getCode()) {

            $requestToPaygate = $payment->getMethodInstance()->generateRequestFromOrder($order);
            $requestToPaygate->setControllerActionName('onepage');
            $requestToPaygate->setIsSecure((string)Mage::app()->getStore()->isCurrentlySecure());

            $dataToDPM = $requestToPaygate->getData();
            $year = $paymentData['cc_exp_year'];
            if (strlen($year) > 2) {
                $year = substr($year, -2);
            }
            $month = $paymentData['cc_exp_month'];
            if (strlen($month) < 10) {
                $month = '0' . $month;
            }
            $dataToDPM['x_exp_date'] = $month . '/' . $year;
            $dataToDPM['x_card_num'] = $paymentData['cc_number'];

            $requestQuery = http_build_query($dataToDPM);

            $ch = curl_init();
            curl_setopt($ch, CURLOPT_VERBOSE, 1);
            curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
            curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, false);
            curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
            curl_setopt($ch, CURLOPT_POST, 1);
            curl_setopt($ch, CURLOPT_TIMEOUT, 30);
            curl_setopt($ch, CURLOPT_HTTPHEADER, Array("Content-Type: application/x-www-form-urlencoded"));
            curl_setopt($ch, CURLOPT_HEADER, 0);
            curl_setopt($ch, CURLOPT_URL, $order->getPayment()->getMethodInstance()->getCgiUrl());
            curl_setopt($ch, CURLOPT_POSTFIELDS, $requestQuery);

            $httpResponse = curl_exec($ch);

            return $this->_processAuthorizenetDirectPostResponse($httpResponse);
        }
  • 4
    Source please...so we know to stay away from it. – Marius Jun 25 '14 at 14:03
  • 1
    I am not naming it at the moment as this is part of a checkout extension where the rest of the code is ok. You will only have an issue if you are using Authorize.net via directpost, which a lot will never come close to. The extension comes with a 45 day return policy. – Kristof at Fooman Jun 30 '14 at 3:27
8

Inheritance 101:

public function _prepareLayout()
{
    return parent::_prepareLayout();
}

From Featured products slider

  • 2
    My guess: Coder wanted a break point for step-through debugging in this specific subclass and forgot to remove the override later. – kojiro Jun 24 '14 at 20:20
  • 3
    OK. Nice one. And I admit. I used it myself a few times. The reasons were different every time. – Marius Jun 26 '14 at 18:17
  • 2
    I think the parent method is protected in most cases so the above code would allow you to use it publicly. Customary would be to use a method name without underscore though. – Kristof at Fooman Jul 24 '14 at 2:13
5

Always do queries inside loop to support sales of full page cache extensions:

foreach($collection as $attribute){

            $option_images = array();

            $attribute_id = $attribute->getId();

            $_option_images = $connection->fetchAll("SELECT `option_id`, `filename` FROM {$table} WHERE `attribute_id` = {$attribute_id};");

            foreach($_option_images as $imageInfo){

                $option_images[$imageInfo['option_id']] = $imageInfo['filename'];

            }

            $attribute->setOptionImages($option_images);


        } 

The code is from one layered navigation extension.

  • 4
    Can you name that extension? – Marius Jul 30 '14 at 4:57
  • 2
    @don't want to advertize competitors :) the module is advanced layered navigation, I think you know them. – Amasty Jul 30 '14 at 14:13
  • 2
    It's not about putting down competition. Of course coming from "Amasty's mouth" it may sound like a hidden agenda, but everyone can see here that the code is just plainly wrong. I think you should add the extension name in here so devs can stay away from it, or at least know the problems. As long as you don't say "Use our extension instead" it should be just fine. – Marius Jul 30 '14 at 14:19
  • 3
    @Marius, haven't seen you comment. the funny code is from "go mage" module. – Amasty Aug 17 '14 at 7:18
2

Something I failed to understand, although it seems strange anyway:

Why there is a unsubscribe from product in a product stock alert e-mail?

  • 1
    I think this answer should go in here: meta.magento.stackexchange.com/q/240/146 – Marius Jul 22 '14 at 6:01
  • 1
    After some investigations I think that this does not belong in the other question either. It's a valid piece of code and a valid feature. See my response. – Marius Jul 22 '14 at 6:22
  • 1
    Thanks. See my comment on the original question. – Niloct Jul 22 '14 at 15:41
2

Make sure, the observer doesn't run if a special block is there.

foreach (debug_backtrace() as $step) {
    if ($step['function'] == 'addOptionsToResult' || 
     (isset($step['class']) && $step['class'] == 'Mage_Adminhtml_Block_Sales_Order_Create_Search_Grid' 
     && $step['function'] == '_prepareCollection')) {
         $addOptions = true;
         break;       
    }  
}
  • can we play "Name that extension?". – Marius Feb 11 '16 at 9:07
  • We shouldn't blame anyone publicly. – Fabian Blechschmidt Feb 11 '16 at 12:51
  • 2
    ehum....yes we should. Public humiliation is the purpose of this thread. – Marius Feb 11 '16 at 12:54

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .