Octarine

2 seconds .. what did you do? Stop and make a cup of tea midway? :P Oh and BTW, you missed at least one other problem :D - - - Updated - - - 2 seconds .. what did you do? Stop and make a cup of tea midway? :P Oh and BTW, you missed at least one other problem :D

Four years ago or so, this was done and dealt with creating a simple expression evaluator that handled basic operators, constants and even functions. It's not difficult to write, it's safe and secure and it works regardless of whether a hosting environment as disabled the eval() function or not.

Yet again, the error message makes it obvious: ` Unknown modifier '=' ` You cannot use the primary delimiters, in this case / within the expression itself. Anything after the second one is assumed to be a modifier.. in this case '=' if (!preg_match("/^(([A-Za-z0-9!#$%&'*+/=?^_`{|}~-][A-Za-z0-9!#$%&'*+/=?^_`{|}~\.-]{0,63})|(\"[^(\\|\")]{0,62}\"))$/", $local_array[$i])) { At a guess I'd say your expression needs simplifying - [!-~]{1,63} perhaps -- just make sure you escape it (mysql_real_escape_string etc) going into the database and escape it (htmlentities etc) going out to the screen.

No, it means exactly what it says Which clearly indicates that the $q is a boolean (as you correctly pointed out), however it needs to be a query resource which in turn indicates that the mysql_query statement itself failed. Defensive programming suggests that you first define a variable to contain the sql statement, then pass that into mysql_query, and finally check the result ie: // Preparation - makes the query below cleaner $pay = $ir['jrPAY']; $strength = $ir['strength']; $labour = $ir['labour']; $iq = $ir['IQ']; $job = $ir['job']; // Define the query $sql = <<<SQL SELECT * FROM jobranks WHERE jrPAY > $pay AND jrSTRN <= $strength AND jrLABOURN <= $labour AND jrIQN <= $iq AND jrJOB = $job ORDER BY jrPAY DESC LIMIT 1 SQL; // Send the query top MySQL and retrieve the result (as a resource handle) $q = mysql_query($sql); // Check the result if (!is_resource($q)) { // If you are debugging, or running on a devlopment box, this is fine to // leave in, but its not wise on production boxes. echo 'MySQL query failed around line ' . __LINE__ . ' of ' . __FILE__ . '<br>'; echo 'Query was:<br>'; echo '<pre>' . $sql . '</pre>'; echo 'MySQL reports: ' . mysql_error(); exit; } // Now .. carry on as per normal. Long winded? Yes, to an extent, but it will nail the bug in double quick time, it also helps if you are using a decent debugging tool like PHPStorm as you can clearly see which lines you can set a breakpoint on and examine the variables at that stage. Code should be designed to help the debugging and testing process after all its *you* how are going to have to read it and fix it when it breaks.

You mean you don't know when somebody is mugged? Oh well, this type of trick works in any number of situations, and I've still to see the need for any in-game crons on any McCode's based system. Server-side crons doing garbage collection, reporting, etc I'll accept, as that is pretty much standard across the board, but another field in the users table and another query to run at some often arbitrary midnight? Now those simple unnecessary imo.

A handy addition, but why bother overloading an already greatly over-used users table, and indeed why even bother with a cron? In authenticate.php, a simple check prior to updating the last_login field would be to check that field against time(). If it is greater than 3 days ago, update the user's job & jobrank fields accordingly and proceed with the normal login process. The same trick can be used for a myriad of things if you look carefully.

Oh I'm so going to have to steal that one .. nice imagery.

include_once('globals.php'); include(__DIR__ . "/globals.php");   What can I say, line 3 and already a problem.   As for the sprintfs' -- Irrelevant. You have failed to understand the reason for using them which while considered a useful trait a few years ago, has mostly been superceeded by cleaner code. -- sprintf('%d', <value>) has proved a useful albeit lightweight mechanism, however sprintf('%s', <value>) is very dangerous when it comes to queries and should be avoided like the plague. -- Since Guest has taken the time to know the underlying database structure - at least data-type wise, and is sensible enough to type-check correctly incoming POST data, there is really no need for sprintf in this instance - which as any good programmer will know is a very expensive function. I would agree at separating the pulling the SQL statements into variables rather than calling mysql_query("...") or $db->query("...") first as it certainly helps if you are forced to run the code through a debugger.   e&oa

I'm sure I should say something really off-color here, but I've known Guest for a good few years, and indeed had the pleasure of his company over a good few libations; all I can say is he's not a bad lad. His code is safe, clean and well written, his knowledge certainly exceeds many on this board and while he may wish to take the lazy way out from time to time, generally sides with common sense, with perhaps a little nudge from shall we say older more experienced individuals. I'm sure the retort will be colorful at least ;)

$brain_file=array(); $brain_file="globals.php"; include_once $brain_file; vs. include "globals.php"; Rocket science its not. Code should be cruft-free, clean, indented, and ideally elegant. (I'll assuming working is a priority), however even this replacement is not safe. include_once(__DIR__ . '/globals.php'); would be a wiser alternative assuming an up-to-date PHP version (5.3.0+)

It may well be going down the tubes, however I for one am happy to to still be working with certain individuals I've met through here - and enjoy supporting some of the younger people by providing services with or without their knowledge, for development, hosting, and simply someone to come to when a problem arises. The ban hammer seldom if ever worked here as more than a few will testify to, neither it appears does the clue bat given some of the recent posts I've read. The owners seem disinclined to take a hand in the organisation of the forum, the administrators equally so, and older posts it seems are rapidly deteriorating as a succession of forum changes have corrupted so many topics. There was a day when the place genuinely flourished, but that being said, there are still some good things coming from it. I see active projects being developed for and by people here with some nice ideas. Clean code, well thought out ideas, standards compliant markup etc., seems almost a shame to tar their release on the same forum as certain recent "ripped" topics. C'est la vie

I cannot help but wonder what the purpose of such a tool is. Code is after all, meant to be read by humans, obfuscating it serves but one purpose - to piss people off. No code is safe from a <strike>truly talented developer</strike> country bumpkin, and as all it takes is one person to de-obfuscate, safety is obviously not a major concern. Correct me if I'm wrong, but didn't that young upstart Ravan use something similar albeit with a callback for his McCodes clone? eval + base64 + gzip - hardly the stuff I'd expect from a web gaming forum.

Impressive

It never fails to amaze me the ability of certain members here to completely fail to understand what is at heart a very basic and extremely powerful concept. For years, we have been have been using multiple connections - application (game) servers each utilize at least two connections to the database cluster which provides read-write splitting, improving ensuring that for example, SELECT statements are handled by read-only database nodes, freeing that load from the primary database. With a very simple replication setup of two database servers; A (master), B (read-only slave); an application could utilize dual connections; $connA for INSERT/DELETE/UPDATE etc, $connB for SELECT etc. Proxies can be positioned easily between the application server(s) and the database cluster extending this concept further and reducing the read load considerably, which can be a stumbling point once games reach a certain size. With careful planning, you can split write load as well. There are other solutions which work on a single connection, but the experience gained from multiple connections is excellent and will hopefully warrant the time spent in developing the ideas. @guest: I swear I've seen that code somewhere else

As a piece of code, it cannot be relied upon for a multitude of reasons, but it served a purpose in its days. Perhaps however, it would be wise to understand the underlying principal of the code, rather than relying on third party (ie MTG) so called "claimants". With some engineering, it can be made to work passably well, assuming you ignore the fact the IP's are not a decent mechanism for detecting multi's - consider for example the small country pub with wifi... "A" likes the game, takes his phone out to the pub, shows "B", "B" registers, instant ban... Why? The solution is relatively simple, and in fact has been deployed in a number of projects including the venerable McCodes, and more recently in an NWE project. Look at a) how you are detecting the IP address, I can almost certainly guarantee you are doing it incorrectly, b) how you store the address, again, it's almost bound to be wrong, and c) how you examine the database for duplicates; that is unfortunately very wrong. It has merit as a bit of code, it does work, up to a point, but making to work properly is a challenge that few have mastered.

A ""Security Alerts"" Forum -- Seriously? It's utterly pointless. I've pointed out holes in almost every so-called ""engine"" here, yet to this day they are still open to multiple levels of abuse. Why? In part because the authors don't understand the problem itself which is fair enough, and in part because they are too lazy to do the work themselves. Look at McCodes. Insecure to this day because the owner(s) are lazy. Pointing out a problem to them earns no credit at all, and even when the identical problem manifests itself in multiple files - pointing out the problem in one file means they fix *that* problem, nothing else. (And even that can be taken under advisement - I can refer you to multiple instances here quite easily.). NWE; well that's an odd one. There are some interesting albeit obscure holes in it (at least as far as this forum is concerned). Alain has either ignored or simply been unaware of some of the basic injection points - though to be fair, I know most if of the problems *have* been secured, in one way or another but initially, there were some questions. EzRPG, lovely gaping holes quite simply ignored by the current author(s) as apparently, they think they know what they are doing. Okay, fair enough, we are all perhaps a little over protective of our code, but really, in this case, any half descent testing would find the problem pretty much instantly. The remainder of the less popular engines - well, 'nuff said. They have all sorts of interesting problems, some of which I've pointed out either directly to the authors or in whatever active forums I've found yet little if anything has been done to address the problems. The answer? Sorry, I can't help you there. "Understand the problem before you ask the question" is probably the best I can come up with, but it's hardly helpful. Security is dreadfully simply when you understand the model you are working with; but so few people seem to be capable of accepting that it is actually outside of their area of understanding. It's actually not that a difficult task to find a hole, it is however a different matter to explain it in terms that can be easily applied to any web application. Advice? I have none - Understanding how insecure data is stored, and then displayed is probably a key trick to learn however and is almost language and database agnostic. Data In -> Escaped -> Database -> Escaped -> HTML rendered. Yet therein lies a hole can of worms, not least of which is encoding and the rather more entertaining database escaping. As for the big guns, the Pentagon etc. Consider the problem. NWE/McCodes/EzRGP - a PHP game running probably under Apache on hopefully a baby Linux system .... versus a (theoretically) military grade system running .. well lets say UNIX to keep things simple. Game ..//.. Military. Really, you don't need that strong security, you simply need the sufficient to protect you, your server and your users. Address the problem from a different angle. Think about the level of control / security you need in respect of the time it takes to recover from backup, repair a lost table etc, repay a lost $3 etc versus spending the next several years working out the next best thing in (for example) password security .. which BTW is *not* PBKDF2 password. (And for those of you still paying attention, scoobie snacks will be award if you can tell me *why*).

Really there is no point in bothering to ban IP's, it's futile. Just looking at the intrusion attempts in the last week on a box I manage - around 320 IP's were flagged as doing something unusual; but is there any need to block them? No - Sure, the few that present a concerted attack on the server - say several thousand attempts per minute were simply firewall'd out - but that should *never* happen at the application level - it's best left to pretty low level stuff. The rest, you can simply ignore. That's assuming you are not one of the usual types here who blindly put up public facing scripts with more holes than a slab of swiss cheese. If you use a decent front end system, then it's own logs can be used to determine intrusion by writing a few short scripts that will perform far better than anything you can dream of at PHP level. In fact, the last thing you ever want is Apache/PHP dealing with anything *but* the application itself, as NWE has some interesting design issues that render it .. shall we say "not optimal" .. for production use. The other benefit to looking at raw server logs is that it becomes an application agnostic solution, something that should be considered strongly in its favor. Look at, for instance: denyhosts. A half decent solution to a particular problem. If you can write something that looks at all requests to a server (note - server, not service), and sensibly block them, then you have the makings of a very decent tool - but, there are some extremely good ones out there already. SourceForge/GitHub - both good places to have a look. There are tools semi built-in to Apache itself, that provide decent logging facilities. There are also some very good tools for looking at Apache's logs, though really, sitting down with Perl for 10 minutes or so is enough to rustle up a script that will flag unusual activity in most log files. You could use an application level IDS but again why bother? Assuming Alain has not gone the remarkably popular route of carelessly leaving huge holes for XSS/SQL - both of which can instantly be considered a game-over event - then is there any need to log/trace/penalize users for making an attempt? If people type in ``<script>alert(1)</script>`` or ``" OR 1 OR login_name="yada`` into a field - possibly some form of mail to another user or a login form, is there any need to penalize them? No of course not. By ensuring you escape the data correctly, they will simply be unable to affect the server in any way other than that which was intended. Yes I agree, there is a case for the "but what happens if somebody discovers a bug in my code or PHP itself" route. Well in your code, that's your own fault. If you can't test it properly before releasing it - or realize that it's wise to have the code checked over then so be it. With a fault at PHP level (or lower god forbid), then yes, logs are handy, but more important is ensuring that your application is well sandboxed from others on the same box, and that you have some form of sane backup and recovery solution. tl;dr: Don't use the application itself to discover, track, log and possibly penalize users for attempting to breach whatever defences you have.

Find them yourself, you don't need to be an expert.

Oh I wouldn't use ezrpg, more holes than a standard mccodes script

Assuming you store the outcome of each round into an array (the individual elements need not concern us here), then array_slice() would appear a handy starting point... $result = array(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12); if (count($result) > 8) { $first_four = array_slice($result, 0, 4); $last_four = array_slice($result, -4, 4); $result = array_merge($first_four, array('...'), $last_four); } print_r($result); Array ( [0] => 1 [1] => 2 [2] => 3 [3] => 4 [4] => ... [5] => 9 [6] => 10 [7] => 11 [8] => 12 ) Though as with most PHP, there are multiple solutions. Which you choose, will be dependant on a number of things, I suspect most notably being the number of anticipated rounds. If this becomes too high, then the combat system may have to be reworked to reduce the load on the server.

Sorry, I think it's time you looked yourself instead of relying on the community to point out your errors. It doesn't take long if you have a keen eye for these types of things, but if you need some help, then enabling error_reporting would be a good start. You stated something earlier along the lines of not wanting to spoon-feed programmers with a fully working, stable, and secure product... Perhaps the community is gradually reaching the stage where spoon-feeding the developers is no longer appropriate.

Addendum Having just noticed the release of McCodes Free 1.1.0b I can say without fear of contradiction that it is STILL insecure. The correction to the code above has been made - but an identical problem exists elsewhere in the code.

Yet plainly, you fail the very people who are willing to provide support. For example: Free 1.1.0 function remove_enemy() { global $ir, $c, $userid; mysql_query( "DELETE FROM blacklist WHERE bl_ID={$_GET['f']} AND bl_ADDER=$userid", $c); print "Black list entry removed!<br /> <a href='blacklist.php'>> Back</a>"; } Free 1.1.0a (After patching) function remove_enemy() { global $ir, $c, $userid; $_POST['ID'] = abs((int) $_POST['ID']); mysql_query( "DELETE FROM blacklist WHERE bl_ID={$_GET['f']} AND bl_ADDER=$userid", $c); print "Black list entry removed!<br /> <a href='blacklist.php'>> Back</a>"; } So the patch which seems to be heralded with a great deal of bell ringing, does ... nothing. Not only does it not address the actual problem at hand, it relies on extremely poor practices - as most of the code does - in coercing data and making assumptions rather than proper type checking. Now I seem to remember a patch a submitted in full being totally misunderstood in the past which is why I am no longer interested in posting complete fixes, however the lack here of anything even resembling a viable patch does indeed suggest an extremely lackadaisical approach to your products and customers. You've stated that you don't have a team specialized in testing, but surely this is the type of testing that can be discovered by both the mark-one eyeball and the very handy error_reporting(-1) statement. Of course the latter would probably scare the pants of you if you've not already tried it due to the large number of errors it spews out throughout the entire code-base. Yes, programmers do make mistakes. But in theory, they also learn from their mistakes. The former is apparent in this, the latter not so. Actually reading through the code - in this case only a few lines - would be sufficient for most people to see and realize that the patch is wrong. Sure, it's an easy one for other people to fix, but herein lies one of the main problems: Simply put, it's not. I've not audited the code, but I can still find a large number of access points simply by performing a quick read of the code. Even were you to have correctly patched this - I'd still raise any number of questions as you have left similar exploits untouched certainly in v1.1.0a. I think perhaps it might be wise to issue a press release stating something along the lines that ""we have been told that the engine is full of holes - so don't expect it to be stable enough to run on a production box as we are not able/willing to fix the problems ourselves"". I'm not sure it will make much difference, since other products do appear to be gaining ground here (at MWG), but at least its being a lot more honest than the current statements. And in answer to the obvious question, why am I not simply reporting the bugs as I find them? Really why should I? Not only does the team ignore certain reports, the muck around with submitted patches so badly as to make it barely if at all worthwhile and in this most recent case, fail to address the problem all-together yet still "ring-the-bell" with a self-congratulatory pat on the back. As Spud stated elsewhere:""what's in it for me?""

Have you actually looked at the code, or are you simply relying on reports and fixing them on a "as-discovered" basis? If you have looked at the code, and can't in fact see the multiple entry points then so be it. I'll consider the entire suite as being simply a learning process for you and the other developers, but if you can see the problems, then I would really question the motives behind providing a product with known problems. I'm sure it probably breaks some archaic rule of these very forums, but I'll not go into that. On the other hand, if you are not looking at the code, then surely you cannot be in a position to defend or indeed support it. 10 PRQNT "hello world" 20 PRQNT "hello world" 10 PRINT "hello world" 20 PRQNT "hello world" See the problem? That's exactly what is happening.

Interestingly enough while the newest patch to McCodes Free 1.1.0a does indeed fix a couple of problems, it remains as insecure as ever. Now, when somebody suggests you have a flat tyre, you fix it. Most people I imagine would have the presence of mind to check the pressure of the other tyres - apparently this does not apply to those people at McCodes who I'm rapidly coming the conclusion actually feel it necessary to provide software that is critically flawed so others may learn. Now there's an interesting marketing model if ever I saw one. Apparently time is in short supply so a thorough analysis of the code has not been possible - which I find rather curious, as it took me less than 30 seconds to discover another unpatched exploited in the so-called patched code and that was by using the mark-one eyeball, not some expensive fool who thinks that automation is the way to find all problems. Now call me old fashioned if you will, but I tend to be of the school where if I install software, I expect it to work. I don't expect critical bugs to exist that can be exploited by any spotty nosed teenager who has ideas above their station and more than 30 seconds to spare... There has to be some reason the staff act like this, though it so far eludes me. Perhaps there really is some misguided concept of providing broken software to try to improve our lot - though I'm not sure who that's aimed at. Game owners after all have to spend time and often money in order to patch the considerable number of existing bugs, so I can't see its aimed at them. Players bear the brunt of broken games with rampant inflation, staff changes, broken (and unnecessary) crons, and in some cases loss of account so again, surely the software can't be aimed to "help" players. As a learning tool it does actually work however; in that it is an excellent aid in how NOT to do things. Maybe that's it. I think the only thing that can be taken away from McCodes is the simple fact that no matter what "they" (the owners) state, you can take with a pinch of salt. It's not secure, it's unlikely it ever will be. Its full of bad practices not to mention areas that still simply don't work as expected. I'd compare it other engines but that would be unfair.