usercheckout.php & userrenew.php - working bad

[darek +]

the listing in the demo disappeared, the test user I created also disappeared, but when I created all, all were functioning when you clicked description AAAAAAAAAAAAA there was alert

I used simple code:

PHP Code:

<B onclick="javascript: alert('Hello Admin')">AAAAAAAA</B> 


[Eric Barnes +]

Just remove all tags from admin -> checkout settings.

[darek +]

Eric, in version, I think, 3.19 you added a very nice cleaning function, it gave me a pain, but I went through it and removed some code that didn't let seller store work nice
why didn't you give it in v4?

to be honest, I got scared when I discovered it, I didn't like cleanhtml.php as it looked for me too complex and I added a function to format.php (I think), it was a part of that class from v3 - it removes all dangerous words/items etc but still leaves the tags, then I use some common tags in 68c admin

I think there should be some sort of protection, otherwise anybody who uses seller store is in danger

[Eric Barnes +]

I will look into this.

[darek +]

Eric, seller store is actually one of the most important thing for me - it has to work, so I need the tags, 68c cleans tags but leaves other things

from

PHP Code:

    /**
     * Code Igniter
     *
     * An open source application development framework for PHP 4.3.2 or newer
     *
     * @package        CodeIgniter
     * @author        Rick Ellis
     * @copyright    Copyright (c) 2006, pMachine, Inc.
     * @license        http://www.codeignitor.com/user_guide/license.html
     * @link        http://www.codeigniter.com
     * @since        Version 1.0
     * @filesource
     */
    // ------------------------------------------------------------------------
    /**
     * Input Class 


I created this function and I put it into Format.php

PHP Code:

function xss_clean($value)
        {                
            $value = preg_replace('/\0+/', '', $value);
            $value = preg_replace('/(\\\\0)+/', '', $value);
    

            $value = preg_replace("/%u0([a-z0-9]{3})/i", "&#x\\1;", $value);
            $value = preg_replace("/%([a-z0-9]{2})/i", "&#x\\1;", $value);        

            $bad = array(

                            'document.cookie'    => '[removed]',
                            'document.write'    => '[removed]',
                            'window.location'    => '[removed]',
                            "javascript\s*:"    => '[removed]',
                            "Redirect\s+302"    => '[removed]',
                            '<!--'                => '<!--',
                            '-->'                => '-->'
                        );
        
            foreach ($bad as $key => $val)
            {
                $value = preg_replace("#".$key."#i", $val, $value);   
            }
        

            $value = preg_replace("#\t+#", " ", $value);
        

            $value = str_replace(array('<?php', '<?PHP', '<?', '?>'),  array('<?php', '<?PHP', '<?', '?>'), $value);

            $words = array('javascript', 'vbscript', 'script', 'applet', 'alert', 'document', 'write', 'cookie', 'window');
            foreach ($words as $word)
            {
                $temp = '';
                for ($i = 0; $i < strlen($word); $i++)
                {
                    $temp .= substr($word, $i, 1)."\s*";
                }
                
                $temp = substr($temp, 0, -3);
                $value = preg_replace('#'.$temp.'#s', $word, $value);
                $value = preg_replace('#'.ucfirst($temp).'#s', ucfirst($word), $value);
            }
        

             $value = preg_replace("#<a.+?href=.*?(alert\(|alert&\#40;|javascript\:|window\.|document\.|\.cookie|<script|<xss).*?\>.*?</a>#si", "", $value);
             $value = preg_replace("#<img.+?src=.*?(alert\(|alert&\#40;|javascript\:|window\.|document\.|\.cookie|<script|<xss).*?\>#si", "", $value);
             $value = preg_replace("#<(script|xss).*?\>#si", "", $value);
    

 $value = preg_replace('#(.*?)(onblur|onchange|onclick|onfocus|onload|onmouseover|onmouseup|onmousedown|onselect|onsubmit|onunload|onkeypress|onkeydown|onkeyup|onresize)[^>]*>#iU',"\\1>",$value);

            $value = preg_replace('#<(/*\s*)(alert|basefont|base|behavior|bgsound|blink|body|expression|head|html|meta|plaintext|script|textarea|title|xml|xss)([^>]*)>#is', "<\\1\\2\\3>", $value);

            $value = preg_replace('#(alert|cmd|passthru|eval|exec|system|fopen|fsockopen|file|file_get_contents|readfile|unlink)(\s*)\((.*?)\)#si', "\\1\\2(\\3)", $value);

            $bad = array(
                            'document.cookie'    => '[removed]',
                            'document.write'    => '[removed]',
                            'window.location'    => '[removed]',
                            "javascript\s*:"    => '[removed]',
                            "Redirect\s+302"    => '[removed]',
                            '<!--'                => '<!--',
                            '-->'                => '-->'
                        );
        
            foreach ($bad as $key => $val)
            {
                $value = preg_replace("#".$key."#i", $val, $value);
            }
            return $value;
        }

then

PHP Code:

case "string":

                
                $NoAllowedTags = '';
                $outputData = strip_tags($value, $NoAllowedTags);
                $outputData = $this->xss_clean($outputData);
                $outputData = $this->IgnoreWhitespace($outputData);


                $outputData = $this->filterword($outputData);
            break;
case "stringHTML":
                $sSQL='SELECT allowedtags FROM '.PREFIX.'checkout_settings WHERE checkoutID=1';
                $result=$db->query($sSQL);
                $rs=$result->fetch();
                $result->freeResult();
                $allowedTags = $rs['allowedtags'];
                $outputData = strip_tags($value, $allowedTags);
                
                $outputData = $this->xss_clean($outputData); 


the IgnoreWhitespace - is another function I had to add as I use short description for google map and too much white space mede errors

function xss_clean($value) - is safe and it works for seller store, some original code I needed to remove from it didn't let seller store work properly

you may want to remember this as it is checked and make seller store work smooth (people like seller store)

[nickhaughton +]

Hi Eric, Darek and all aboard,

This thread seems to be wandering a bit from its point and I'd like to add my experience as a the owner of a 13-year-old newspaper/magazine which moved it's on-line classifieds system to 68 dev 4 three months ago. I, like Darek, am not a programmer but I have had to pick up a bit of php along the way to do small tweaks and to write a routine to extract all our approved ads in their sections, apply some formatting and prepare them for insertion into our monthly paper publication of which we currently churn out 15,000 examples.

Congratulations to Eric and the team on the neat code. It's a pleasure to wander through.

Right, so here's my impressions about the order process after 3 months with 68:

First off, we manually accept payment and approve paid ads, and to be honest I don't trust the system to handle payments smoothly yet.

It took me a while to get my head around the orders, renewals and listings concept and even though I know it makes sense it still seems clunky to me. I have now learned to work with orders instead of listings. But let me give you the example of the clunkiness I mean:

I come in on Monday after a weekend of people placing ads and find myself with 40 new orders, say. I scroll through the first few pages to find the oldest awaiting approval order on page three. I click on the order. I click on listings tab within the order. I click on the listing within the listings tab and then I edit the ad to correct mistakes and save the changes. 68 returns me to...

...the manage listings page 1. You could say that makes perfect sense, but that doesn't help me out. I click on "Orders" again, then "manage orders" again. Scroll through page one and two again till I get to the oldest order and change it's status to "Approved", hit submit and 68 returns me to...

...page one of the orders. I scroll through the pages again (inspired by the fact that I've only 39 orders left to process) and arrive at the last awaiting confirmation order. Hmm. There are four of them from the same user. This time I right click them and open them in new tabs to avoid so much travel. The first one has no listing in it although it is listed as a paid ad. The second, third and fourth are a similar ad with a time difference of 1 minute and 20 seconds respectively between submissions.

I select the most recent one after verifying it is the best, and discard the other two orders with "delete". But the blank one I leave well alone. I have learned the hard way not to delete these, even if they are throwing the money income tally out of sync for the rest of the month, because, just like Darek discovered, they can be connected to a live listing and when you zap them you zap the users ad which is embarrassing and time consuming. So I make it approved too and forget about it. Close all the tabs. Submit changes and back to page one.

I will probably come across at least another 6, but sometimes many more duplicated orders before I get home to page one, and these are all people getting confused with the order placement process, firing browser back buttons, closing explorer, panicking, opting out... whatever. And each time it generates me an order to investigate and probably a couple of emails for the client too.

I think it is well worth a long serious think / brain storm about how it might be possible to fool proof this rather soon, while we admins still have hair.

Finally I hope this is constructive critisism and I don't come off as a moany old turnip getting his gripe in. I really appreciate the product, and I am aware that it tries to serve a wide spectrum of users and that one persons issues may not be those of another.

Darek is so right when he suggested that people like to twiddle, dabble, poke and say awww and then close their browser (and not always in that order). We should let them do it and only pass the whole mess on to us when it is cooked.

Keep up the good work.

[Eric Barnes +]

Hi Nick,

I personally try to listen and take in any advice customers have to share. That is the biggest reason for us getting to the point we are now.

I think I have posted this before but just to be clear I am very happy to seriously consider an alternate approach to any of the internals. As long as it keeps the same feature set we already have.

Now I am not saying the way the checkout system is the best but from what you describe all shopping carts have this same issue. We constantly get orders that are not completed, they changed there minds, etc. But this is duplicated in your admin because orders and listings are separate. Which I think they should be. This way the order system can be used for other things in the future.

[darek +]

Nick, this is excatly what I noticed!
Eric, all the confiusing comes from breedcrumbs, it is there to use it, but when it is used we get unfinished order and from unclear buttons.
First - step one, then step two then listing is created and user can edit the listing in normal way but the order is NOT created, the user or admin can delete it without getting into problems.
Then when user wants the listing he can press the button "order the listing" and the order for this listing is created.

This way we would only have to deal with payed or unpayed listing without all confusing that come from info unfinished, uncopleted or whatever.

All the issues come from 2 tables listings and orders but they are not created at the same time, if it is too difficult to deal with both lets deal with one only.

[nickhaughton +]

Eric,

I haven't delved inside the order stages, but from what Darek is saying it seems that the order is created before the end of the stages. Is that essential? Is that not where the problem comes from? That the order is created and later the user bums out...

Excuse my ignorance

[darek +]

Nick, that is correct,
I think I remember, when you are on step 2 you chooce the package, once you press go on, the empty listing and order are created then the next steps just fill in the tables with data, that is why I insisted on at least renaming buttons as they are unclear and users don't expect to create anything when they actually do that. Also deviding order from listing would let everyone know what is going on. The idea of the entire system is not that bad, but joing it all together is not the best option.