will this script be completely rewritten from scratch?

Discussion in 'General Support' started by interdummy, Apr 24, 2012.

  1. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    ..by a competent programmer, or at least somebody who knows how to code?

    I mean, look at this:

    Code:
    function unsetsessions()
    {
    	global $_SESSION, $system;
    
    	$_SESSION['SELL_with_reserve'] = '';
    	$_SESSION['SELL_reserve_price'] = '';
    	$_SESSION['SELL_minimum_bid'] = ($system->SETTINGS['moneyformat'] == 1) ? 0.99 : '0,99';
    	$_SESSION['SELL_shipping_cost'] = 0;
    	$_SESSION['SELL_file_uploaded'] = '';
    	$_SESSION['SELL_title'] = '';
    	$_SESSION['SELL_subtitle'] = '';
    	$_SESSION['SELL_description'] = '';
    	$_SESSION['SELL_pict_url'] = '';
    	$_SESSION['SELL_pict_url_temp'] = '';
    	$_SESSION['SELL_atype'] = '';
    	$_SESSION['SELL_iquantity'] = '';
    	$_SESSION['SELL_with_buy_now'] = '';
    	$_SESSION['SELL_buy_now_price'] = '';
    	$_SESSION['SELL_duration'] = '';
    	$_SESSION['SELL_relist'] = '';
    	$_SESSION['SELL_increments'] = '';
    	$_SESSION['SELL_customincrement'] = 0;
    	$_SESSION['SELL_shipping'] = '';
    	$_SESSION['SELL_shipping_terms'] = '';
    	$_SESSION['SELL_payment'] = array();
    	$_SESSION['SELL_international'] = '';
    	$_SESSION['SELL_sendemail'] = '';
    	$_SESSION['SELL_starts'] = '';
    	$_SESSION['SELL_action'] = '';
    	$_SESSION['SELL_is_bold'] = 'n';
    	$_SESSION['SELL_is_highlighted'] = 'n';
    	$_SESSION['SELL_is_featured'] = 'n';
    	$_SESSION['SELL_start_now'] = '';
    	$_SESSION['date_work_all'] = '';
    }
    When this will suffice:

    Code:
    unset($_SESSION['SELL_array']);
    I'm not entirely sure which version I have but I have never seen so much damn right harmful code in a single application. So what if it is opensource, that isn't an excuse for putting out trash like this.

    I'm not simply bashing here - this code, at least the version I have, has so many glaring security holes it only takes a clueless script kiddy to hack my server. You wanna send a link to your site with this script? If you have installed the version I do, I can hack your site within minutes.

    PLEASE developer, have some sympathy for your community and have somebody completely rewrite this junk!
     
  2. Guest

    Guest Guest

    interdummy welcome to webid world
    we all just try to do our best. :)

    Like to get some professional written secure code.
    so how can i change it to

    PHP:
    unset($_SESSION['SELL_array']);
    mutch shorter, why is it better.
    and is it more secure ?
     
  3. nay27uk

    nay27uk Super Moderator Staff Member

    Joined:
    Nov 24, 2009
    Messages:
    3,918
    Likes Received:
    265
    Not sure Dahl but I presume you replace all of the code he posted with just unset($_SESSION['SELL_array']);
     
  4. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    That is correct. It is far more efficient code. Not just that, you are using $_SESSION['SELL_variable'] = ''; to unset. This means you are still storing $_SESSION variables on the server until they expire. You should unset() and completely remove them from memory.

    I appreciate you are just trying to do your best, but for the sake of your users, I urge you to put this into 'beta'. It is their websites who will be hacked because of this software, not yours.

    Another almost unacceptable piece of design is the categories table. You have:

    cat_id, parent_id, left_id, right_id..

    and the children of a given cat are between left_id, and right_id. This is so unbelievably bad I'm finding it hard to even make adjustments to the code (for a client).

    What you should do here is have a matrix table which contains a cat_id and a parent_id. Makes simpler and more robust to manage categories then.
     
  5. Guest

    Guest Guest

    code is not build from scratch its build from closed project called simple auction and it had lots of bad code, and as you found out there is even more bad code, love the way we can remove bunch of code lines change them in to one line of even better code that is great.
    when we fix things we try explain and make examples telling what files involved, before after code and a nice topic that explains content so it is useful when doing search in forum

    I took from here your info and code using it as example, made it into a thread you can see here,
    for making nice color code i use the php icon in the advanced editor and i set the flag on top in editor to webid version (1.0.3)

    Session variables is not removed from memory

    hope i have understand what you said and made it right
     
    Last edited by a moderator: Apr 29, 2012
  6. Xeonn

    Xeonn New Member

    Joined:
    Dec 15, 2011
    Messages:
    372
    Likes Received:
    54
    interdummy you enter here with angry on whole world as this script is dangerous. as you see this project is developed by many users, give an advice hide agression :p .
    I think that you are f**king noob , and your words which you said
    only proved this opinion.
     
  7. pani100

    pani100 Well-Known Member

    Joined:
    May 9, 2011
    Messages:
    1,743
    Likes Received:
    291
    Well maybe its not all unset because we have not finished with the sell information.
    If we look at the code it is not unsetting the variables anyway it is actually setting them. Don't get fooled by the actual name of the function it could of been called anything. Have I missed anything here?
    If we have fees on live mode it would take you to pay page, but also the final sell page has 3 options , view auction (which is fine because it is just a link to the auction) but also edit and sell similar which need the new set array to work.
    Also
    $_SESSION['SELL_minimum_bid'] = ($system->SETTINGS['moneyformat'] == 1) ? 0.99 : '0,99';
    $_SESSION['SELL_is_bold'] = 'n';
    $_SESSION['SELL_is_highlighted'] = 'n';
    $_SESSION['SELL_is_featured'] = 'n';

    They are not actually asking to be unset are they?
     
  8. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    What are you talking about? This script is dangerous. I'm not here for a an argument. No experienced developer could defend this script, you realize that right? There are that many holes.

    It isn't hard to see you're a little offended because you think I'm some kind of troll. I'm not! I came here after my clients site was hacked due to this script, and I've been cleaning up the developers mess every since.

    Refrain from being an immature cry baby - throwing personal attacks will only perpetuate the issue. Somebody with maturity and objectivity (AND EXPERIENCE) needs to take a long hard look at this script, and redesign from the ground up.

    I can keep reeling off bad code if you want, but there is so much of it, you really need to just get the message: 90% of this code isn't usable in a commercial environment. It has cost my client a lot of money! I'm not just being difficult here.

    I was trying to help..but w/e, you're the experts right :D

    ===

    settings table..column for each setting?! Should be a simple key => value pair - 2 columns, infinite rows, ability to easily add further settings etc

    Not seen ANY use of mysql_real_escape string to escape the data or even validate it. Thankfully, have typecasted some of it.

    Personally, I think the bad design starts from the database and has forced some bad design in the code because of it.

    I'd advise you continue to use OOP where you can and slowly convert everything over to a more expandable system.

    What about search engine friendly URL's? What about integrating facebook/twitter login? What about trying to change any core part of the application without having the entire house of cards come crashing down?

    I really wish you the best with the script, but I can't see how this script can be taken seriously without being completely rewritten - and you have plenty of customers you can inform of this, you could even have a free and paid version, or maybe the paid version could be the rewrite, I don't know..but one thing you should stop doing is adding more bad code.

    And users who come in here and start throwing personal attacks at me for pointing this out and trying to help (if I didn't care, I wouldn't be here) need to grow up! If you can't take constructive criticism then don't put out software for the whole world review.
     
  9. Xeonn

    Xeonn New Member

    Joined:
    Dec 15, 2011
    Messages:
    372
    Likes Received:
    54
    sorry for being so angry, but we only see here requests from users, and there are only few people involved in developing this project.
    So I get angry when you wrote about code.
     
  10. pani100

    pani100 Well-Known Member

    Joined:
    May 9, 2011
    Messages:
    1,743
    Likes Received:
    291
    Well if your clients site got hacked and you wanted to help you could of easily told us immediately about the event and how it was done to get us working on securing it. I cant see any thread about "my site got hacked everyone be careful".
    And if you read the about tab this script is in beta mode. Help and ideas are always welcomed here.
    There is only 1 developer of this project, currently AWOL and we are just trying our best with what we have
     
  11. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    I see where you are coming from. I am sorry to be harsh, I hope my suggestions will help in the development of the script. It is still a HUGE achievement to have got the script this far, and would be a shame to allow the lack of some important patches to get in the way of that.
     
  12. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    With all respect, I'm not about to spread my clients details on a public forum, or even privately, considering I am a developer myself and have since plugged the holes (or what I could).

    One step in the right direction is replacing the current mysql connection with PDO, or mysqli, and using prepared statements. This will resolve most of the security holes. Would also be a good idea to move all of the tables in the app into objects, like you have with user, and then setting validation rules within those objects to ensure the data isn't compromised in/out from the database.

    It might be a good idea to check into OOP PHP, and have a look at the MVC architecture. You could even use an MVC framework like codeigniter and you'd have some very good code to begin with.

    Again, I'm sorry I was so harsh, it is just you have to take responsibility for the potential problems (especially financial) your software causes to its users.
     
  13. nay27uk

    nay27uk Super Moderator Staff Member

    Joined:
    Nov 24, 2009
    Messages:
    3,918
    Likes Received:
    265
    interdummy.

    You seem to be an inteligent individual and seem to know what your doing with regards to coding php and sql.

    As many others have pointed out everyone that is on this forum are just users of this forum that help one another fix bugs and make mods ect with the exception of only one member whome is the author of the script (yes he is a one man outfit).

    WeBid has come a long way since it started life as simple auction from a rewriten verssion of PHP Auction GPL, the authour always does his best to release fixes for any known security vunerabilities and tries to support the script to his full ability.

    What the project needs is someone like you that is competent with PHP and SQL coding to help better the script and share that with us all, the hole community.

    I urge and ask kindly, that with all of the modifications you have done for your client, that you upload a copy of that script along with all of the safer code modifications and share it with us all.

    I understand that no two coders code the same but if just one coder like you shared his knowlege and the code he had rewriten then this would give the authour of the script a solid foundation to work with.

    I speak for myself the authour and many of the members here when I challenge you to please upload the hard work you have done and share it with us all, WeBid could be a fantastic script with people like you helping better the code.

    So will you take up the challenge and upload what you have already done
     
    Last edited: Apr 29, 2012
  14. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    Ok, my final list of suggestions to bring this script into modern times:

    1. replace mysql_ with mysqli, PDO, and use prepared statements:

    http://php.net/manual/en/book.pdo.php

    2. convert each table into an 'object', and use the object to build validation rules and maintain data integrity

    http://www.tonymarston.net/php-mysql/databaseobjects.html

    3. rethink the caching. Should be automatically refreshing parts of template based on changes to original files:

    http://stackoverflow.com/questions/156552/creating-a-two-pass-php-cache-system-with-mutable-items
    http://simas.posterous.com/php-data-caching-techniques

    4. check the code into source control (if not already):

    https://github.com/

    5. use ORM - object relational mapping - (as 1 above) - this will allow for easy ways to insert fixtures and other test data, and share that between other developers:

    http://readthedocs.org/docs/doctrine/en/latest/en/index.html

    6. write unit tests for core parts of the application involving financial matters:

    http://www.simpletest.org/

    7. read through best practices for php and review app at same time, recording instances of bad code:

    http://net.tutsplus.com/tutorials/php/30-php-best-practices-for-beginners/

    Hopefully that will be enough to help developers turn this script into an even better script.
     
    Last edited: Apr 29, 2012
  15. Xeonn

    Xeonn New Member

    Joined:
    Dec 15, 2011
    Messages:
    372
    Likes Received:
    54
    hahahahah good advice :) read here, and here and here and try yourself :)

    give us 2-4 major files to get it compare . than we could see what direction we must go. It won't break your financial status, but if you think that someone will pay for you if it's not nessesary - you are wrong ;)

    by the way, what makes difference where I'm from for you ? as you posted above?
     
  16. nay27uk

    nay27uk Super Moderator Staff Member

    Joined:
    Nov 24, 2009
    Messages:
    3,918
    Likes Received:
    265
    Well thanks for your input, although not a PHP coder I will be having a read through the links you posted and I belive that the authour will to as will most of the more regular users of this forum.

    Hopefuly you will stick around and maybe post some of your code improvements as actual code and share them with us all
     
  17. Guest

    Guest Guest

    interdummy no need to tell who your client is for sharing your code or telling in forum about problems with script.
    nice list of links but think sharing the the solutions you come up with is what we most need. and some info about how would be nice too. and think you would gain something out of it too we all do mistakes in code and can find yours (if any ) and help you too
    sharing and posting code is always win/win situation

    ;)
     
    Last edited by a moderator: Apr 29, 2012
  18. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    I feel those points I mentioned are the most important, but I think the last link with best practices is a great article that every php developer should read. I'm not sure what you mean by financials..but developers should be rewarded for their time. There could be paid themes, plugins, features etc. with still a large portion of the application always free..

    To be honest, this isn't my place to insist something be done. I just hope the information I've pointed to and the suggestions I've made can help in the future.
     
  19. interdummy

    interdummy New Member

    Joined:
    Apr 24, 2012
    Messages:
    28
    Likes Received:
    1
    To clarify my position: I am actually very very busy with my own projects. I'd love to stick around for a while and help where I can, and have others help me if need be. You are obviously a very nice bunch of people and very welcoming. However, I just have too much work on at the minute to spend any great amount of time working on this project with you.

    I will surely be back in the future to see how development is going. It is nice to people who are open to improvement, as I am myself.
     
  20. Guest

    Guest Guest

    lol was as i expected from day one you wont give your code away
    just some hints about webid is messy and you are good programmer,
    wee see types like you a couple of times each year.
    actual it take less time sharing development in forum and getting work done, as we all can help where we can, than sitting with it all alone by yourself,

    ---
    we have discussed payment before and i am totally against it,
    if people want to support webid or the developers they can donate all they like,
    there is donation link to webid on top of page and donation buttons for coders in the download section,
    , paying for themes mods languages etc is killing the idea of open source, if we should get paid webid should now cost 1000 of dollars, i work with webid around 15 hours each day and has done that since Dec 2009 so if i should sell my theme i doubt anyone would pay for what i have spent on developing it, and we all have used code from each others so if i would sell i had to discuss it with every other coder,
    same i guess goes for every other developer. and if there should be a pay version as you suggested, Who should be on that pay roll, every coder that have been with the project cause we all have our part of it, or what ?
     
    Last edited by a moderator: Apr 29, 2012

Share This Page