Mabinogi World Wiki is brought to you by Coty C., 808idiotz, our other patrons, and contributors like you!!
Want to make the wiki better? Contribute towards getting larger projects done on our Patreon!

User talk:Saiyr

From Mabinogi World Wiki
Revision as of 19:19, 9 October 2011 by Xcelled194 (talk | contribs) (My script)
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to: navigation, search

My script

Hey, have you finished looking at my script? I'd like to approach IJ about getting it hosted on the wiki, and it'd be great if you could say you've read it, and can't find any malicious stuff, ect. He'd still want to look over it, but perhaps be more receptive Xcelled194 17:44, 2 October 2011 (UTC)

I looked at it a bit, but not in a great amount of detail. I think what you have is okay, but not ideal. I think it would be better if a cron job did the server pinging and updated a database and the script just read from it. It means load time will always be fast for the wiki and it won't be writing to the file system. I don't consider writing to the file system like you do to be a good idea if it can be avoided. Saiyr 21:34, 2 October 2011 (UTC)
For what it's worth, your "ideal" configuration is almost exactly like the script:
  • Cron jobs
    • Not really needed in this case. The script loads faster than the wiki main page when reading from the cache, and even when it's not, it still returns in ~3 seconds, plenty fast. Also note that iframes/ajax are asynchronous, meaning the wiki will never be waiting for the script before it finishes loading.
    • Cron jobs might actually slow the wiki down. Whereas now, the script runs on demand, a cron would have it constantly running.
    • However, setting up a cron job would be very easy. I'd just modify the script to cache it in an html file, then set the iframe to load that. The cron job would just run my script.
    • This is more a question for ij
  • Write to a database
    • A database is just a fancy file, so either way, you're writing to the filesystem
    • You'd have to query the database every time. This would massively up the server's CPU. You should hear Marck at MM b----- about their queries. It is much faster and easier on everyone to use a local file
    • Ij would have to set up a database for the script, and he would have to do a lot in terms of setup/maint..
    • The script's portability would be removed. Right now, any webserver with php could run my script off the shelf, no issues. A database would complicate that. (Think future server upgrades)

That's just my two cents on the issues you pointed out. I suppose another option is to always have the script echo the cache, and then update the cache if needed. Thus, preceived load times are instantaneous.Xcelled194 04:07, 3 October 2011 (UTC)

A cron job that ran every minute would not slow down anything, especially if, as you say, the load time is 3 seconds. The delayed load time when the file cache expires is pretty obvious even if it's still relatively fast. I'm well aware that a database is a "fancy file," but the web server process should not be writing to the file system or have write permissions if it can be at all avoided. A database would not massively up the server's CPU usage. A simple SELECT query is a drop in the pond compared to the queries Mediawiki will generate for everything. Portability isn't really relevant to me. Setting up a DB shouldn't be a big deal, and IJ should not have to touch that DB after it was setup. The cron writing an HTML file would solve the issue too, but it's not the most secure option either. Saiyr 04:24, 3 October 2011 (UTC)

I'd be more than willing to set the script up for a cron job (it pretty much is already), however, that's IJs domain , and is up to him in the end. As for the database, it seems to me like a violation of KISS, it's just one more (unnecessary) thing to go wrong. Plus, using a database, I'd have to rewrite the script (again), execution times would be longer due to the script querying every time, and IJ's workload for implementing goes from copy/paste a few files to setting up/configuring another database. As for the 'security' concerns, I don't think they're really a problem because:
    • I specify the name of the file in a string literal, meaning it'd be impossible for someone to change the save location.
    • I preform a lot of sensitization on user input.
    • For the most part, the script calculates its own values, minimizing the use of user impact.
    • The user input section is on the wiki, and is restricted to registered users with more than 10 edits, Joe Blow can't come in and wreak havoc.
    • Because of the wiki, any hacking attempts are recorded in the history for posterity.
So, in my mind, as long as IJ keeps PHP up to date (which he'll have to do anyway, for mediawiki), if someone manages to get around all of the above, IJ has bigger issues that need to be addresses, and a database won't help any. It seems to me like a lot of extra work for very little (if any) gain to use a database. Xcelled194 14:21, 3 October 2011 (UTC)

This argument is going in circles. If you feel it is ready to send to IJ then send it. Just know that security concerns go well beyond your script once it has write permissions. Saiyr 15:41, 3 October 2011 (UTC)

Gosh, I hope I didn't come across too strong. I welcome your input (and, in fact, want it). I'm giving my views on the issues you raised, but that's just my view. I'm not trying to pprovoke/bash you, I'm kind of thinking out loud, and giving my objections/thoughts. You've hardly told me any of yours, for example, why is a database better/more secure? Who knows, I might beswayed Xcelled194 18:35, 3 October 2011 (UTC)
A database would be better because the server status is stored in a structured format that is easy to validate. If the script cannot write to a web-readable file, there is less chance for an attacker to use whatever user the script runs under to write malicious code. This is a little less relevant after talking to IJ, due to the setup of the wiki, but I still wouldn't be okay with it if I were him. Like I said, just e-mail it to him and see. In the end my opinion doesn't matter. Saiyr 18:43, 3 October 2011 (UTC)

Actually, your opinion DOES matter. It matters a lot. You're an admin here, and your stance will likely impact IJs stance. In addition, this is peer review of my script, a time when I'm examining it for flaws and seeing what can be done better. (Some, like the database, I remain unconvinced of the benifits to that, but I'll see what IJ says). So, what would you say if I implemented a very strict filter, only allowing the values listed on the config page (ie. online, maint, 1, ect) and simply filtered the rest out. Since PHP auto-escapes quotes in strings, an attacker couldn't "bypass" the filter. The only side effects here are additional processing (probably not an issue) and if we ever need to add a new status, we'd have to modify the script, whereas before, we'd just drop the new image file in the script's directory and it'd work. I'm thinking it'd be a good thing to do. OH, and I'm tweaking the script slightly to be an instantaneous response always, as well as making it cron-friendly. Xcelled194 20:11, 3 October 2011 (UTC)

I'm actually only a wiki admin here to take care of something that I keep putting off (implementing timers on the wiki). I am normally only an IRC admin. I have already explained my opinion to IJ and he doesn't seem to think it's a big deal (though I haven't gotten a definitive answer in either direction). You don't necessarily need to whitelist the user input if you feel it's too strict, but I'd say only allowing alphanumeric statuses might be a good idea. I don't know if you do that already. Saiyr 20:14, 3 October 2011 (UTC)
I do a similar thing, courtesy of PHPs internal sanitation methods, however, a Regex that filters on alphanumerics is a good idea.... [implements] Xcelled194 20:29, 3 October 2011 (UTC)
Here's the result of my regex-matching. Given the input of hi=3&foo=bar&nope=', the script filters it down to this: hi=3&foo=bar Xcelled194 21:26, 3 October 2011 (UTC)
Out of curiosity, why are you putting ampersands in the wiki template and parsing it as a query string? Saiyr 22:06, 3 October 2011 (UTC)
The ampersands allow me to use PHP's built in function called parse_str(). I first have it parse the query into an array, which I sanitize. Once its sanitized, I re-call parse_str() with no argument. This actually creates the variables in the script's scope, meaning I can use them without going through the hassle of parsing them myself. Also, using an & makes it very easy to separate statuses in code.Xcelled194 22:17, 3 October 2011 (UTC)
Parsing it without the ampersands wouldn't be that much harder in my opinion, but it's up to you. It just looks strange when you edit the file and see them there "for no reason." Saiyr 22:21, 3 October 2011 (UTC)

(Going to un-indent this again) I could use some other symbol, but in addition to the ampersand being what PHP uses natively, if we removed the ampersands, and someone accidentally put in two end of lines, then you'd wind up with funky results (I tried that at first[Also, due to system line ending differences, do you match at \r, \n, or \r\n? Everyone's different...]). This way, people are very unlikely to stick an ampersand where it doesn't belong. Xcelled194 22:29, 3 October 2011 (UTC)

I don't think newline type matters in most cases, but it isn't difficult to handle in any case. You don't need any symbol besides a newline, and it also isn't really difficult to handle double spacing. If you think ending everything with ampersands instead of newlines will make sense to everyone that edits it, then leave it as is. All I'm saying is you should consider the editors, not the simplicity of the script. Saiyr 22:37, 3 October 2011 (UTC)
The newline type matters a lot. On Mac, its \r. On Linux, its \n. And windows, not to be left out, does \r\n. The script can only match one. So my point is, no matter which ending you choose, you're leaving someone out. You could correct this by normalizing all the line endings, but if you're not careful, you'll actually end up with MORE line endings and/or an infinite loop. Eventually, you'd just have to change them all back to ampersands anyway. Lol. Plus, what if someone forgets about a line ending? They'd break that status, as well as the one below... If it becomes an issue, I'll look at fixing it, but so far, people have been doing OK with it. All they (really) need to know is that every status is separated by an ampersand.
This whole newline thing is just a mess, as I discovered when I attempted to use it originally. Xcelled194 22:49, 3 October 2011 (UTC)
Did you try preg_split with the PREG_SPLIT_NO_EMPTY flag? Saiyr 23:01, 3 October 2011 (UTC)
No, I'll have a look at that... Xcelled194 23:03, 3 October 2011 (UTC)

preg_split (or replace) would work, however I feel that if the ampersands aren't causing trouble, they're probably beneficial because they make parsing easier, they tip off editors that this is no longer the manual edit page and they need to use different conventions. I'll keep it in mind if people start complaining, but for now, I'll probably just leave it as it is. Xcelled194 14:33, 4 October 2011 (UTC)

Just conceived of a (imo) great security addition to the script. Since parse_str() replaces any variable, it's possible for someone to change a critical variable accidentally. Therefore, follow me, we call strtolower() on the configuration string. Any variables we do not want changed we simply start with a capital letter. :D Xcelled194 00:54, 10 October 2011 (UTC)

A better security feature would to use the second argument of parse_str and read the variables you need into the global scope. Your idea is simply security by obscurity. Saiyr 18:38, 9 October 2011 (UTC)
Partially, but I actually do use the second parameter:

parse_str($Config, $Vid); $Config = ""; foreach ($Vid as $Key => $Value) { //Here, we do your idea of alphanumeric args, which we then store back in Config } $Config = strtolower(rtrim($Config, '&')); parse_str($Config);

I could set up a global array, but that opens up its own can of worms (plus makes the script harder to maintain, and I'd have to parse at least two array levels every time I want to do something), so it seems to me like this is an effective compromise? Xcelled194 00:54, 10 October 2011 (UTC)
I don't see how it becomes harder to maintain, but that's not relevant. Simply make sure the final key is something you definitely want before putting it back into Config. Saiyr 23:06, 9 October 2011 (UTC)
It makes it harder in the event we need to add something, plus it's confusing if you have like Input["alexina"][0] or more. Xcelled194 00:54, 10 October 2011 (UTC)

Just a note, not sure if I'm conveying what I want to here. I'm saying, by forcing the configuration string to all lowercase, we can prevent people from overwriting things we don't want. This works because the non-overriable variables have at least one uppercase letter, which, due to PHP's case-sensitivity, makes them impossible to change from the all lowercase user input... It's not security though obscurity at all. We're hiding nothing. I could post the source code, and the lowercase security measure would still work perfectly. This security is, in effect, placing a wall between the important vars, and the status vars. Let me know if I'm still not making sense. Xcelled194 00:54, 10 October 2011 (UTC)

I understand quite well what you were saying and I am telling you it is not a good idea. It is a very silly way to improve security and only works if anyone who modifies it remembers to follow this convention. Saiyr 02:08, 10 October 2011 (UTC)
I fail to see how it's silly. It does a great job of limiting what a user has access to within the script. Combined with the other sanitization, it seems to me that the script is pretty secure. Yeah, if this was the only thing, I'd be uncomfortable with it. Xcelled194 02:14, 10 October 2011 (UTC)
Then do what you wish. Saiyr 02:17, 10 October 2011 (UTC)
>_> I'm trying to understand why you think it's a bad security measure D:
If you didn't understand what i said before: Your idea requires anyone who would modify your script to use this non-standard convention of capitalizing variables so it doesn't get overridden. This naming convention is not enforced in anyway and someone who was unaware of this may decide to use lower case variable names like most people do and suddenly it can be overridden. If you understand this and disagree, I have nothing left to say. Saiyr 02:37, 10 October 2011 (UTC)

What if we changed it to uppercase? I have a big NOTE of documentation at the beginning of the script also. You're right that if someone is careless enough to disregard the warning, it would open the script up to attack, but I think we have three things going in our favor: 1) The attacker would still need to see the source code, or be very, very lucky. 2) The script should need to be edited very rarely, if ever. 3) If someone is enough of an idiot to not read the documentation, chances are IJ would never let them near the source.

Also, I think we should take a step back, and look at the overall picture. This isn't some high visibility company that's fending off hackers everywhere, nor is it obvious how the script works. The worst the wiki has is a few bots and vandals. Basic santization and policing should be enough to ward off any bad eggs and script kiddies. If we get a real hacker, IJ's going to have a lot bigger problems than the script (Not that the script wouldn't be a target, just that s/he could probably find 1,001 other ways into the server). My point is, at some point, we need to acknowledge that there's a point where additional security is not needed, and it ceases to become beneficial (violates KISS). I'm starting to wonder if we're not already at that point. What do you think? Xcelled194 02:52, 10 October 2011 (UTC)

We are never going to see eye-to-eye on programming. I do not think that the usage frequency of a script determines how secure or sloppy a program should be, nor do I think the alternatives to your idea violate KISS. Simply do what you think is best and let IJ look at it. I am no longer interested in discussing any aspect of this script. Saiyr 03:11, 10 October 2011 (UTC)
Never said "sloppy". It's a matter of how far do we go. Do we spend time protecting against something that is very, very unlikely? So far, IJ hasn't responded to my thread or email. I'll go to IRC next. Thanks for your input though. Btw, I credited you for your idea of filtering non alpha-numeric values, as well as a general reviewer. Xcelled194 03:17, 10 October 2011 (UTC)

Just FYI, another program I made

I left the post on Angevon's Talk. Xcelled194 01:55, 5 October 2011 (UTC)