Feature - BBCode PHP Processor

[Luke]

Ok... How else am I going to explain this... Ok lets go over the feature first, which I have no problem with, and generate steps that would lead to disaster. Lets begin.

The "BBCode PHP Processor" allows you to parse BBCodes with PHP code so you can do dynamic things with BBCode, such as color PHP code on the fly. I think this is an easy concept to understand, so let's move on.

Let's imagine that you can import the PHP file through an xml file.

Step 1) Attacker gets into your AdminCP through some unknown security hole.
Step 2) Attacker destroys your forum's database and puts in "Haha you suck" messages all over, and "L33t" crap all over the place.
Step 3) Attacker isn't satisfied. He wants to destroy your backups, shell_exec is enabled on your server, and base protection isn't enabled because you don't know any better (oops). He decides to upload his own PHP code through the BBCode import feature that's conveniently there for him.
Step 4) He then executes this code by using the "BBCode Test" feature also conveniently in AdminCP. His "custom code" wipes every file on your server. If "BBCode" test wasn't there, he could simply post that code somewhere else. The code doesn't necessary have anything to do with BBCode or returning anything, it has a mission of destruction.

If the code had to be uploaded via FTP, then they would have to also have access to your FTP to do this. And if you're the only one with FTP access, they can't upload their nifty destruction script.

Have I made my point YET?

[Digi]

Right. Except one problem. IPB's group settings have been doing this for years now without issue ;) With all the mods out there adding ACP settings, why hasn't someone come forth and exploited that? Perhaps they have. Who knows. The bottom line is that you don't install things from untrustworthy sources and you won't have an issue.

I see no one complaining that someone hacked them by importing settings. I see no one complaining how much easier it is to install settings in the ACP. You are saying "this is a bad idea" off of silly assumptions here. Name one vulnerability that has ever plagued the ACP and allow someone without access to access it. (note: I said "plagued the ACP" not "some exploit that allowed an admin user's password to be reset [thus implicitly giving access]").


And finally. If some one ACTUALLY got in to the ACP. There is no need to do anything like this. All they need to do is go to the nifty "SQL Toolbox" and ruin your life.

[Luke]

The "raw php code" in the settings is parsed to ensure some commands are not used. As far as people exploiting it: That's as far as you know. 90% of the issues that take place are brought up through the ticket system and handled by the staff. Many customers don't even use the forums. (And I'm not saying many issues are specific to this, I mean in general).

Another thing to note is that IPS has added a settings plugin feature to replace the "raw php code". Even though that the "raw php code" thing still exists, I don't see it staying in the next major version. The biggest reason why it still exists is for legacy support.

Yes some attacker getting into AdminCP can ruin your life through SQL Toolbox. But there are two things I want to bring up:

#1) If you make backups you're safe. If you don't have your server setup securely, executing PHP code can eliminate that safety.
#2) Since IPB 2.2 certain queries are blocked unless IN_DEV mode is turned on (if I remember correctly).

[Digi]

The "raw php code" in the settings is parsed to ensure some commands are not used.

Why couldn't this same thing be used for bbcode? I never once said a file "should" be created automatically based on X data submitted (though I did suggest it as an option). I used the settings as an example of how to implement this for bbcode.

If it is being phased out, that's fine. I find it sad that such a useful feature (the ease of use considered here as well) is removed simply because "something could happen". If you have a properly configured server, not even your backups are at risk. How long until someone finds a way to exploit the skin html logic system and we lose that? What about an ajax exploit? Anything can happen with programs. That is just the nature of the beast. You have to weight the cost with the benefit.

Regardless of how the php is handled, I think this is a feature that should be strongly considered for addition. :thumbsup:

[Jaggi]

i tell you what digi your an experienced coder so tell me which is easier and quicker, to code a entire system the will encode a PHP file and convert it to xml which is also a lengthy stupid and needless process. Or to have a php file (LIKE the profile portal AND components system AND the portal and simply upload a php file via a ftp. The idea of this mod was to open the bbcode for more manipulation and customization so how is this achieved if your distributing data around that is all encoded and converted to xml??? You mention the other systems that use xml and encoding, for example acp settings, skin settings. None of those are executing large bits of php, they are just settings in set format which is easily checked to make sure it follows a standard and if it fails so does the import of the script. Currently there are no functions in ipb that allow the import of large parts of executable php so why change that? its that idea in itself that causes problems and starts punching holes in security. Also what if my bbcode has hundreds of lines of code and a couple of functions and some other nice bits which i'm sure will be a standard thing if this addition goes ahead. Its just so much simpler and easier to have a php file you upload via a ftp and just send it off to someone else or your live board than to have to worry about converting it to this that and the other, you idea has no benefits what so ever.

[Digi]

First of all. Take a deep breath. You are getting far too worked up over a topic that, frankly, it seems you may not understand the detailed workings of and perhaps I've not done well enough in explaining. :)

PHP (or anything text for that matter) can be serialized and transported as an XML safe string with one function. It isn't even completely necessary to make it safe with the use of <[[CDATA type syntaxes in XML, but it can be done with one function, base64_encode. Remember, php is text until otherwise provoked.

Please give a valid example of a bbcode that will actually require more than 20 lines of code. Nothing that I can fathom should or would require any serious processing outside of a loop or minimal text manipulation spanning 1 to 20 lines (though lines are subjective information and not valued to the actual content of code on such lines).

Lastly, for your attack on "no benefits". As Luke has noted, for all new settings that will use the legacy php bit in the acp, you will soon be required to upload a file for simple processing. This means that for things like checking for values set by multi selects, etc you will need a small plugin file containing the code. Here is an example of what I mean:

if ( $save == 1)
{
    if ( is_array($_POST['warn_protected']) )
    {
        $_POST['warn_protected'] = implode(",",$_POST['warn_protected']);
    }
    else
    {
        $_POST['warn_protected'] = "";
    }
    
    $key = 'warn_protected';
}
if ( $show == 1 )
{
    $key='warn_protected[]';
}

You want me to put that in a file every time I want to use it???? Do you know how many exact clones (minus key names) of that exact line of script exist for settings manipulation in the ACP?

(Note: the above is mildly subjective because a core class could be used to for that example, but it was used merely to prove a point here)

We have to do this now because it makes the possibility of an attack using this system less likely. So IPS is doing this because something "could" happen. While this is a good thing to some degree, if we were to design everything based on what "could" happen, you wouldn't see half the features available in IPB. I'd rather see IPS focus on preventing something from happening rather than saying "we shouldn't use feature x because if something else is compromised something 'could' happen." Especially when the benefits of the feature outweigh the likelihood of something that "could" happen. Either way, I support IPS's decisions. It is a fine line to walk between usability and security. Anyone that develops any program hopes that when push comes to shove, their product will trip to the secure side of the line. ;)

How does this hurt? It hurts because it is yet another file that must be properly uploaded. How many times have you heard people with issues that turned out to be a corrupt file uploaded via ftp or permissions not being set properly? The more files there are to upload the greater the chance of this occurring. More so, bbcode doesn't currently require ANY files to be uploaded via ftp. The only thing required to upload is an XML document containing the bbode (same with settings until, likely, the next release). Requiring this means that much more hassle just to install a single bbcode like PHP syntax highlighting.

A quick note, I actually made a mod that required only one settings.xml file to be uploaded and one single skin edit. It took 4 points totals set by other settings in the ACP and ordered them in to a html output. All this was done via the ACP taking advantage of only using the settings php bit. The html output was dynamically updated anytime a point was changed and it produced an output like this:

http://www.ipsbeyond.com/forums/index.php?...post&id=241

So what is the benefit to my idea? The simple fact that the entire bbcode XML document can be uploaded via a field in the ACP and added to IPB's database. No extra processes, no extra files. One, streamlined, process for the whole thing. It means less problems for the user, less confusion on what to do, and one process for the whole thing regardless of the type of bbcode added. Given your version of the idea, for a normal bbocde (like centering) the user would only have to upload an xml file. Then, for something like php syntax highlighting, the user has to have an extra step of uploading a file. You know how many topics a day I see on IPSB about "what is ftp?", "how do I upload files?", etc? People need simplicity, and if the possible risks don't outweigh the benefit of providing that simplicity, which they don't here, it should be something that is considered.

A mild example here. Imagine that the component system allowed for one file to be uploaded via the acp and it did all the work for you. Every file installed, every setting uploaded, everything else taking care of. While bbcode may be far more inferior to this example, it is one in the same on a smaller scale. You can't tell me you do not see the benefit of doing something like that. Further, would you be happy about seeing that feature taken away because something "could" happen if some other rogue feature screwed up?

Anyway, in a previous post I noted that my idea is good, but not necessary. I want the ability to use php plugins of some form for bbcode. So regardless of how it's done, I want the feature. My views on how to implement it are simply there for IPS to consider when they actually start to develop it in hopes that they'll make it as easy and accessible to use as possible for all users, regardless of understanding of server architecture, programs, or files. :)

[Luke]

The first thing I would like to comment on is the settings plugin vs raw php code. With the settings plugin if you have more than one field with the same type of processing you do NOT have to have a new file or function per field. You can use the same plugin file to do the same task in several places. With the raw php code, as un-secure as it is, you had to copy the same code more than once. The settings plugin, from what I remember, is passed into a single function and gives you information on what fields are being processed. You could easily use one function on several fields of the same type. All you need to do is keep track of these fields in an array and when it's process identify it.

The second thing I would like to bring up is uploading files through an xml import... If setup correctly, yes it would be easier than uploading a file through FTP. But in order for it to work each folder would be need to be chmoded correctly. If you had five different imports that would be 5+ folders you would need to chmod. If you want to talk about support.... Chmod is one of the most complex easy things with a forum. Tell a customer they need to chmod something to 777 and many will be like "what the heck does that mean?". More than likely if you tell them to upload something they'll know how to do that.

Another point I would like to bring up is these custom plugins are 3rd party. If you added an import feature for everything and they needed to chmod something to make it work it would be IPS's problem. If they can't figure out how to upload and install a 3rd party plugin, it's the authors responsibility not IPS's. I highly doubt IPS is going to go to that much trouble for something that would easily be done through FTP, and at the same time add to their support load.

On a security note: The only way I can see to make this somewhat secure is have a confirmation password stored in the config file specifically for this. So when you select the file you have to put this password in (disabled by default).

[Jaggi]

ok heres my reply, do you know how many hours of my life i've wasted away manually and tediously writing up xml files and putting each of the tags in and doing the cdata stuff? i should sue ipb for this time (don't worry guys i'm not gonna). It also takes away the core thing that this entire addition is meant to provide which is flexibility, sure its not a problem for us because we're seasoned coders but for someone trying to get into providing a solution you've just made the entire thing more complex.

Secondly you argument against uploading php files was weak, i mean the corruption part especially... like xml files never get corrupted, lol!!! And also files get corrupt more via a http upload than a ftp and thats a fact!. Next thing is your proposing upload the files and dumping it into the db... well its php so it still needs to be outputted a php file eventually so either creating some files using the cache system which basically just does what uploading a file does!!! or select the data from the db on the fly while running the parser. Problems with this are, firstly all data is inaccessible and again this mod is meant to make things more flexible!!! Secondly your running unneeded sql queries which could easily be avoiding by a simple include. Remember even tho php files are constantly updated for performance is really the sql db we all bend over backwards to keep happy. Again your method would all multiple queries that are totally unnecessary.

Your benefit was you can upload a file to the acp without any extra headaches, great but to do that you've already had to go through three extra steps to prepare the file. Simply having a bbcode directory and a sticking a php file in it and pointing the acp setting to that file is so much easier and is the tried and tested AND existing way ipb does things. If it ain't broke...?

P.S. the imdb bbcode modification is more that 20 lines ;).

[Digi]

The first thing I would like to comment on is the settings plugin vs raw php code. With the settings plugin if you have more than one field with the same type of processing you do NOT have to have a new file or function per field. You can use the same plugin file to do the same task in several places. With the raw php code, as un-secure as it is, you had to copy the same code more than once. The settings plugin, from what I remember, is passed into a single function and gives you information on what fields are being processed. You could easily use one function on several fields of the same type. All you need to do is keep track of these fields in an array and when it's process identify it.

I don't know exactly how the settings plugin works. Keeping this in mind though, just because a single file (as far as you reveal) COULD be used multiple times does not mean that moders out there know if you have setting file X installed, nor that they won't replace it with their own settings plugin file. Thus, even though the database scheme takes up space, the file scheme could take up a lot of space as well, even if it is redundant.

The second thing I would like to bring up is uploading files through an xml import... If setup correctly, yes it would be easier than uploading a file through FTP. But in order for it to work each folder would be need to be chmoded correctly. If you had five different imports that would be 5+ folders you would need to chmod. If you want to talk about support.... Chmod is one of the most complex easy things with a forum. Tell a customer they need to chmod something to 777 and many will be like "what the heck does that mean?". More than likely if you tell them to upload something they'll know how to do that.

You are right, CHMOD is up there in the top support issues. However, places like cache folders should, and are usually (or you'll find other issues) already chmod. All new files created from the script will then be properly chmod without issues (example skin/language cache files). However, CHMOD issues will not go away as they are inherent in IPB for things like skins, language files, style_images, etc. Once a chmod issue is fixed (which like I noted, in the case of the cache folders, if you have an issue with one file there, most likely all will be diseased) it usually remains that way.

Another point I would like to bring up is these custom plugins are 3rd party. If you added an import feature for everything and they needed to chmod something to make it work it would be IPS's problem. If they can't figure out how to upload and install a 3rd party plugin, it's the authors responsibility not IPS's. I highly doubt IPS is going to go to that much trouble for something that would easily be done through FTP, and at the same time add to their support load.

No one said by giving the ability to allow uploads meant it was IPS's responsibility. If that were the case, they'd already be responsible for any emote, bbcode, settings, skin, or component xml import that already exist. By allowing php to be included in to these imports does not give IPS any more responsibility, nor does it require that things function (as far as chmod, files allowed, etc or anything outside of "working as intended") to the user.

On a security note: The only way I can see to make this somewhat secure is have a confirmation password stored in the config file specifically for this. So when you select the file you have to put this password in (disabled by default).

Don't we have this already to some extent? You must login to the ACP. You must also have permission to see the page that allowed uploading, editing, or adding item X. Adding an additional security code (especially to a clear text document) will only cause more usability problems. Not only that, but any one that is actually able to access the ACP will more than likely have a way to get the required information to upload these files. Finally, if you have a shared password between admins (not saying you do), what is to prevent a silly admin from uploading an infected file? (or any admin for that matter) These things just happen. However, for the sake of argument, I'm all for an additional security measure (setting in the security options) that allows one to create a password that would be hashed and stored in the settings table. This password could be used as an additional protection against things that allow any access to raw data (ie SQL Toolbox, uploading any file, etc). :thumbsup:

ok heres my reply, do you know how many hours of my life i've wasted away manually and tediously writing up xml files and putting each of the tags in and doing the cdata stuff? i should sue ipb for this time (don't worry guys i'm not gonna). It also takes away the core thing that this entire addition is meant to provide which is flexibility, sure its not a problem for us because we're seasoned coders but for someone trying to get into providing a solution you've just made the entire thing more complex.

No, please do not compare, what I can only assume, your experience creating a modification installer XML file with that of any XML file that IPB creates. These files are created automatically anytime that you ask for them in the ACP. You can make settings.xml files by turning your forum IN_DEV and going to the main settings page. At the bottom is an option to export ANY setting that you want. You can export any skin and images to an XML file automatically. You can export all emotes, bbcodes, and so on, automatically to and XML file. You can't blame IPS for your time, you can only blame yourself for not using the wonderfully full featured tools available.

Secondly you argument against uploading php files was weak, i mean the corruption part especially... like xml files never get corrupted, lol!!!

If you actually want a statistic, though XML files do "screw up" from time to time, they 99% of the time screw up on the DOWNLOAD/COMPILE portion. After that, every user that uploads that file will end up with a corrupt file. This is a far different experience than the occasional ftp "oops". In your case it affects all users and is easily recognized. In my case it is random, uncommon (overall), and not easily recognized.

And also files get corrupt more via a http upload than a ftp and thats a fact!.

When a file is corrupt on upload you get a nice error message. In fact, it says "The file appears to be corrupt. Please try again". When compared to an ftp "oops", one is easily recognizable, the other is not.

Next thing is your proposing upload the files and dumping it into the db... well its php so it still needs to be outputted a php file eventually so either creating some files using the cache system which basically just does what uploading a file does!!!

First it doesn't HAVE to be output to a php file (look up eval ;)). Additionally, CREATING a file on the local system in comparison to internet protocol file upload and transfer is a no brainer. Things get lost in TCP/IP, data gets corrupt. However, this doesn't happen on the local system (at least not near as commonly as via TCP/IP). As Luke noted above, most issues with writing files will be CHMOD related rather than file related. If we use the cache system that is in place now, the likelihood of CHMOD being related to ONLY bbcode upload is extremely slim.

or select the data from the db on the fly while running the parser. Problems with this are, firstly all data is inaccessible and again this mod is meant to make things more flexible!!! Secondly your running unneeded sql queries which could easily be avoiding by a simple include.

How is the data inaccessible? Just like settings currently are, you can access it from the ACP. Just like skin templates, when IN_DEV is set, an option could be available to allow importing from flat file (guess you didn't know that either? :S). I don't understand how the data is inaccessible.

Remember even tho php files are constantly updated for performance is really the sql db we all bend over backwards to keep happy. Again your method would all multiple queries that are totally unnecessary.

The purpose of php cache files is to reduce DB load. If DB load wasn't an issue, even the skins would be stored there. So storing to file is not going to make anything bend over backwards. The IPB system for file creation is already expandable for this instance. I used it myself to create mod files using my Mod Injector. It isn't built solely for skins.

Also, though it likely wouldn't work out for bbcode, the option of a cache_store array is available too. Just like settings, forums, etc the data could be stored in an array that is only called during parser operations. Thus reducing the SQL load as with everything else. So again, I don't understand the SQL bend-over-backward assault.

"Your benefit was you can upload a file to the acp without any extra headaches, great but to do that you've already had to go through three extra steps to prepare the file. Simply having a bbcode directory and a sticking a php file in it and pointing the acp setting to that file is so much easier and is the tried and tested AND existing way ipb does things. If it ain't broke...?"

What is broken?? What are you even talking about? 50% of the things in IPB use a flat file direct upload method another 50% use the XML upload method. The latter is the tried and tested method, not the former. Portal plug in files, components, login auth (though I personally see this as an exception), and, soon, settings are all newer and not as thoroughly tested as the xml files (though does it need to be? I don't think so). Not to mention that PHP in settings.xml files has worked and been tested and used since 2.0 ;) All are perfectly viable and useful ways to get the job done. I'm just suggesting to side with usability over lack there of.

Three extra steps? I still don't get your angle. Where are the 3 extra steps? There only 3 (basic) steps for the moder in the first place. There definetly isn't 3 steps for the end-user. Here's an example flow of operation for you. Note that this operation is pretty much exactly the same as the, soon to be, legacy settings operation.

-=Moder=-
1) Click "Create New BBCode"
2) Fill out typical bbcode form (one that already exists) and add php to be used for parsing to inline editor.
4) Deploy

-=End User=-
1) Upload file
2) Use bbcode

Notice. Never once did I say "manually build XML file", "create a php file manually", or "ftp in and continually update php file during testing". Where's the extra 3 steps?? Let's not even begin to compare initial install of XML vs. flat file to upgrading. If you split the 2 and require new bbcodes to use xml and php flat file, you have 2 things to do (like components) not one. More trouble in the long run. With a flat XML file you need only upload one file each time and the data is over-written.

"P.S. the imdb bbcode modification is more that 20 lines ;)"

Not only is that modification over bulked and inefficient with unnecessary code for the job, but it has to take a little more in order to MODIFY the current IPB code. By using a system like this, all the data would be streamlined and less code needed to complete the transaction.

In closing, I don't quite understand what or why you are even arguing with me on this. Not only does it not matter. I've already said that it isn't necessary. I've already stated that it isn't the focus of this request. I've also said that I simply added my suggestion for implementation should IPS want ideas. You have every right to refute that, but you are just going in circles fighting a battle that doesn't exist here. I'm not saying "IT MUST BE THIS WAY", just giving an idea for a simple, modular approach that will remain consistent throughout all installations of bbcode (whether php is needed or just a simple center tag).

:)

[Luke]

Anything that requires a forum owner to CHMOD a directory (not file, directory) to make a feature work in the first place is IPS's responsibility. Beyond that if they had to upload the file they would have to consult the author, not IPS. IPS would not upload the file for them. Let's put it this way.... If they didn't know how to CHMOD the directory IPS would have to do it for them. If they had to upload the file, IPS isn't going to upload the file for them. The best they can do is tell them where to upload the BBCode module.

If, and I stress If, the feature was made the best way to secure it would be a confirmation password stored in the config file. If it was stored in the database they could simply access it or change it through SQL toolbox. That would defeat the purpose. You would have it in the config file because even though they get AdminCP access, by what ever means, they would not and should not have access to PHP to read the variable, or the file where it is stored.

Last thing I want to bring up is eval. Eval is a command no programmer should ever consider using for any reason. It is true it is used for the raw php code, but as I stated before that code is filtered and I believe the "raw php code" is on the way out. It is never good practice to use eval, ever.

There should never be a way for an admin to input php code from the admin panel. That's just asking for trouble. If someone get's into your admin CP, the last thing you want them to do is execute a script.

[Jaggi]

ok just going down in order you replied here. Firstly i get what you mean with the xml creation, i do use it but still find i have to do some editing on alot of things still, thats irrelevant here tho. lets just assume thats all fine and works and move on...

Actually if you use a decent ftp it will tell you that a file is corrupted. Also about half the time if you try to upload a corrupt file depending on how bad it is it can just crash your browser. Another thing is that usually files are packaged in a zip/rar wotever and they themselves check for corruption, so as i said before this is still a weak argument and doesn't really carry any weight. Also your error message sucks "your file is corrupt please try again?" thats more confusing cus if its corrupt why would you try to upload it again? :P.

I agree with luke here eval isn't a nice function, wasn't it 2.1.3 or something (cant' remember right now) that got hacked the crap out of cus of a eval function?? Also don't know where you've come up with all thise tcp/ip stuff because no0ne mentioned that and you seem to have forgotten you still have to upload a xml file so your issues could effect you, sure your CREATING a file but again like that never causes problems, data can get corrupt in a db, your server could die in the middle of creation process causing a failed cache file, all these things apply to you too.

yes i know about the flat file from import while setting IN_DEV mode so i guess thats another step... i guess i'll just write that down. So if we want to change a small thing we have to set IN_DEV and upload the file again... nice.

Ok your database point is valid but as your pointed out various cache solutions aren't available for bbcode. Also your gone into talking about reducing sql load... erm upload a file would wipe out sql load all together cus you wouldn't have to run one query unless required by your actual bbcode mod.

Obviously your not used to various expressions "if it again broke... don't fix it" either your just trying to be picky or you've never heard of that expression which is just scary :P. Flat files are more tested then xml, your forgetting ipb is one big collection of flat files, although this is generalisation xml is the new addition to the coding world and the feature currently under testing which sticking a chuck of php into a file and executing it pretty much the way its always been.

Ok i come down to your last bit where your saying we should have a text box inside the acp where just throw chunks of php into? so in order to edit something or add something and remember i'm being realistic here, you'd copy the code into your text editor (no0ne can edit or write php in a text box, thats a fact) edit, hope it works and then paste it back into the text box and submit it, then you can go off and check to see it works as opposing to uploading a file from your editor and click refresh in the browser. Then we have the other problem, OMG theres a syntax error in your code that you missed!!! ok this always happens, just a typo or something... but the code has been shoved into a db, outputting to a cache file or called via a eval function... now it doesn't sound like its going to give a very accurate response to errors. Right we'd know its a problem with our acp bbcode, but for lesser experienced seeing an error saying that there's a problem in bbcode_parser.php on line 51 and all you see is a eval function... or bbcode_cache_2.php so you have to trawling through the ftp looking for this file. or if you have errors off all together you just get a blank page :D.

The imdb mod or the coder is not in despute here it was just an example and the integrity of the code is not up for discussion here so we'll let that ship sail (thats another expression by the way).

Who's arguing? i just consider this to be a healthy discussion, healthy because it picks a quite large idea into focus and outlines the benefits and disadvantages which is a democratic thing... i am simple putting my idea forward to ips as an alternative which again as a customer is my right :).

[Digi]

Anything that requires a forum owner to CHMOD a directory (not file, directory) to make a feature

work

in the first place is IPS's responsibility. Beyond that if they had to upload the file they would have to consult the author, not IPS. IPS would not upload the file for them. Let's put it this way.... If they didn't know how to CHMOD the directory IPS would have to do it for them. If they had to upload the file, IPS isn't going to upload the file for them. The best they can do is tell them where to upload the BBCode module.

If a directory within the cache folder already present is used, as I've said numerous times here, then if you have a CHMOD issues you are going to have more than just one problem here. Once that is resolved, all those problems AND this will be moot. So while CHMOD may be a problem to IPS, it is not their responsibility and storing files within directories that are already supposed to be set up this way will cause no more load for IPS than skin files and language packs.

If, and I stress

If

, the feature was made the best way to secure it would be a confirmation password stored in the config file. If it was stored in the database they could simply access it or change it through SQL toolbox. That would defeat the purpose. You would have it in the config file because even though they get AdminCP access, by what ever means, they would not and should not have access to PHP to read the variable, or the file where it is stored.

First, not only did I say the password would be encrypted in the database, but I also said that it would be required for accessing anything that has the ability to process raw data. This, as I suggested, means the SQL Toolbox, uploading of files and anything else that allows access to raw file/sql data (ie php in settings). Now, sure, you could store the same hashed password in the config file rather than the database. Frankly, I feel the database is far more secure and harder to gain access to than any single file on the server. Not only that, but the config file is at least readable to someone that can find the file. The database is not, in the simplest of ways.

Last thing I want to bring up is eval. Eval is a command no programmer should ever consider using for any reason. It is true it is used for the raw php code, but as I stated before that code is filtered and I believe the "raw php code" is on the way out. It is never good practice to use eval, ever.

Eval, just like any other user input method, is a perfectly acceptable function to use. The only time things like eval, dl, and exec get a bad rep is when the programmer didn't properly sanitize the data and/or used it when it was completely unnecessary to do so. Just think now of all those sites running in safe mode. If it wasn't for eval, they'd not be able to use IPB ;)

There should never be a way for an admin to input php code from the admin panel. That's just asking for trouble. If someone get's into your admin CP, the last thing you want them to do is execute a script.

The same could be said for javascript. So, I guess we should remove the template manager from IPB. Concidering that JS is usually the leading cause of XSS and account theft, I'd say that it is by far more of a risk than php. If we are going to chastise php, we should DEFINETLY be chastising JS.

Actually if you use a decent ftp it will tell you that a file is corrupted.

You are confusing contexts yet again. Firstly, now you are saying that, to solve IPS's corrupt upload problems, all customers should be required to have a "proper" ftp program. Second, unless your ftp program is running md5 checks both locally and remotely against the uploaded file, there is no way that it will know if the file is corrupt. A size check will not tell you this. Nothing will except a hash check. No ftp program that I know of can or will do this. FTP as a protocol doesn't even support a function for hash checking a file. So again, no, ftp cannot and will not tell you if a file uploaded is corrupt. It will tell you if the upload failed for various reasons (one not being corruption unless it is so extreme that the files size changes) and it can tell you if a file downloaded is corrupt (bad binary headers in a file or just plain retarded) or failed, but it cannot tell you, especially in our case, if a specially formatted XML file is corrupt.

Also about half the time if you try to upload a corrupt file depending on how bad it is it can just crash your browser.

That is one annoyance, true. This is a browser issue best I can tell though (FF in particular here). Even if this is the case, if 15 users in a day have that problem, we can successfully relate it to the file. In the FTP the file case, if those same 15 users happen to have an FTP corruption, the likelihood of them all getting the same error is very slim. Some will see bbcode simply "not work", others might see parse errors, others...who knows. Nothing consistent and nothing that points directly to a corrupt file.

Another thing is that usually files are packaged in a zip/rar wotever and they themselves check for corruption, so as i said before this is still a weak argument and doesn't really carry any weight.

There is no checking against the downloaded xml or xml.gz files that IPB allows to download. The checking that you are talking about is actually done via the program (ie winrar) opening the file. What do rar/zip files, much less client side checking that isn't available in IPB, have to do with IPB at all? Let's just say for 1 second that it did have something to do. That corruption check is done upon opening the file. Not uploading it, not downloading it, not anything else. Only opening it. So how does that affect anything that I've pointed out? Are you trying to grab at straws here?

Also your error message sucks "your file is corrupt please try again?" thats more confusing cus if its corrupt why would you try to upload it again? :P.

Not that it doesn't need help perhaps, but it isn't as simple as I or you made it sound. From memory it says "Your file appears to be corrupt, please try your upload again". "Appears" is a strong word in this case as it shows that it is possible that something happened via HTTP transfer, but to try again. If the issue keeps happening, you know there is something wrong with the file. I did some testing to get the full text used when importing an XML file and it appears that this error message has been changed, for the worse. Hopefully that can be fixed.

that got hacked the crap out of cus of a eval function??

Uh...no I don't believe that is correct? Either way, eval is perfectly fine when used correctly as noted above.

Also don't know where you've come up with all thise tcp/ip stuff because no0ne mentioned that

Actually, you did. By using the argument that FTP and local file creation are one in the same with file corruption. I'm sorry if you don't understand internet foundations, but this is fact. FTP uses Internet Protocal (aka TCP/IP) to do what it does. This is far more susceptible to data corruption and loss than a local system running a program. If you don't understand internet topology, how can you even argue with me about what one can and can't do with it?

"and you seem to have forgotten you still have to upload a xml file so your issues could effect you"
At least during upload you KNOW if the file was either corrupt to begin with, or happened to become corrupt during HTTP upload. We KNOW this because we can check it properly, because IPB KNOWS what an IPB file should look like, and can shoot of an error message if it doesn't match up (corrupt or not...read..hack attempt). FTP cannot offer this same level of usability.

"sure your CREATING a file but again like that never causes problems, data can get corrupt in a db, your server could die in the middle of creation process causing a failed cache file, all these things apply to you too."
Data cycling can resolve many of these issues, and IPS has slowly moved a lot of processing to that. However, most of these issues are far beyond IPB and point to underlying configurations on the server. The same issues can happen during a flat file installation given the need to install itself to the database.

"Ok your database point is valid but as your pointed out various cache solutions aren't available for bbcode. Also your gone into talking about reducing sql load... erm upload a file would wipe out sql load all together cus you wouldn't have to run one query unless required by your actual bbcode mod."

You are talking about something completely different here. Now you are suggesting to move all custom BBcode to a flat file. Even those that are only html change overs. I can see the storage costs going up already, not to mention the mindless unnecessary data processing for something as simple as [ center]. BBcode is and will always be stored in the database. The main use for bbcode is to give users access to x/html code while maintaining security. While, if this were to be implemented, some bbcodes would need some minor php programming, the bulk of all bbcodes will remain html (as they have been for years upon years).

"Obviously your not used to various expressions "if it again broke... don't fix it" either your just trying to be picky or you've never heard of that expression which is just scary :P. Flat files are more tested then xml, your forgetting ipb is one big collection of flat files, although this is generalisation xml is the new addition to the coding world and the feature currently under testing which sticking a chuck of php into a file and executing it pretty much the way its always been."

I know exactly the expression you were using. As I tried to explain last time you used that, XML is not new, it has been around for many years now. It has been used by IPB for MANY years now. It is tried. It is tested. So there is nothing broke in either case. There is nothing to fix in either case. XML has been around longer than PHP! ;) XML | PHP

"Ok i come down to your last bit where your saying we should have a text box inside the acp where just throw chunks of php into?so in order to edit something or add something and remember i'm being realistic here, you'd copy the code into your text editor (no0ne can edit or write php in a text box, thats a fact) edit, hope it works and then paste it back into the text box and submit it, then you can go off and check to see it works as opposing to uploading a file from your editor and click refresh in the browser."

No one can write php in a text box? What is the difference between a text box and word pad or notepad? Ability to use tab....that is it. Further, there is no difference between using a local text editor and uploading the file manually via ftp than there is to using an internal IPB text pad and hitting submit, especially if it is going to a flat file.

"Then we have the other problem, OMG theres a syntax error in your code that you missed!!! ok this always happens, just a typo or something... but the code has been shoved into a db, outputting to a cache file or called via a eval function... now it doesn't sound like its going to give a very accurate response to errors. Right we'd know its a problem with our acp bbcode, but for lesser experienced seeing an error saying that there's a problem in bbcode_parser.php on line 51 and all you see is a eval function... or bbcode_cache_2.php so you have to trawling through the ftp looking for this file. or if you have errors off all together you just get a blank page :D."

I'm well aware of the issues surrounding development and issues with eval and line numbers for execution (although eval does spit out the line internally that failed with it if the text is submitted properly ;)). This, however, is exactly why I mentioned the "sync with flat file" option that skin templates have. The same idea could be used for developers that need that "line accountability" during initial development of bbcodes.

"The imdb mod or the coder is not in despute here it was just an example and the integrity of the code is not up for discussion here so we'll let that ship sail (thats another expression by the way)."
Don't bring up examples if you don't want an intelligent response back. You gave the example of some mod that takes more than 10 lines. I told you exactly why I think it takes that much. Regardless of whoever made it, I feel that way. The same thing goes for many mods I see today. So don't moor in my docks if you don't want my inspectors to board ;)

[Luke]

Not only is storing PHP code in a database and evaling it a bad idea, it's completely unnecessary.

I want the BBCode PHP processor, but only if I can simply upload a module and specify the file name. Doing anything else would over complicate things.

The database is accessible before a file on the server is. If a confirmation password, as I mentioned, was stored in the database it could easily be changed.

[Digi]

Not only is storing PHP code in a database and evaling it a bad idea, it's completely unnecessary.

I never said it was necessary in most cases. Just that it is perfectly viable in others, hence the safe mode skins example.

I want the BBCode PHP processor, but only if I can simply upload a module and specify the file name. Doing anything else would over complicate things.

So again, I ask you. Please tell me how, out of these 2 scenarios, your option is the less complicated one.

Usage, module upload:
1) upload module file via ftp
2) install bbcode.xml file in acp
3) set module file in acp (this could even be omitted and included in the xml file)

Usage, current bbcode method expanded with php included in XML import:
1)install bbcode.xml file in the acp

The database is accessible before a file on the server is. If a confirmation password, as I mentioned, was stored in the database it could easily be changed.

The web server is the front end of all data execution from anonymous sources on the web. Thus, the files are the first thing available. The database can only be accessed through files giving them access or through direct access via a mysql client with proper authentication.

This is my last reply here as I see us starting to work in circles here. My suggestion is made, yours is made. I would like the system implemented regardless of how it is done as I see it as a viable asset to the community. :thumbsup: :)

[Luke]

So you're complaining about adding one extra step: uploading one file via FTP? ...

First you said the module itself should be imported through XML... As I said the plugin directory would have to be CHMOD'd. Now you want the PHP code saved in the database and cached using mem cache... That is unnecessary and ridiculous. I saw more logic in your first idea of importing the plugin file through XML. But as I told you before, being able to import a PHP file through AdminCP has it's risks if someone got into the admin panel.

The database is the first and most likely place they would get into before a file on the server. If they got into admin CP, they would have access through SQL toolbox. If they found a SQL injection vulnerability, again they would have access to the database. The only way they are accessing the source code of a PHP file on the server is if they had FTP access, or some how they already executed server side code.

I'm not going to repeat how bad of an idea it is to allow someone the ability to execute PHP code within any web interface. The only way it should be done is through FTP. And by doing that not only do you simplify the code itself, you eliminate having to support it other than "you put the 3rd party plugin in this directory". Also, what you're suggesting is more work, when all we really want is the "BBCode PHP Processor" bit. I don't know about you, but I could care less on how it is imported as long as it is secure.

And eval... There's a saying that goes something like this: If your solution is to use eval, you haven't considered all the solutions.

[Antony]

In case you didn't notice administrators can insert PHP code into the settings config area. I don't see why we need to use eval(), I'd personally use create_function().

[Luke]

As I stated that is the only place you can insert PHP code, but it's filtered and the "settings plugin" feature replaces that. I believe the only reason why it still exists is for legacy reasons.

[Jaggi]

well not going to bother to reply to digi's post, he's right it is going round in circles and these long posts tiring me out :P, so no point making a further argument especially if he's not going to reply, we both have valid arguments i just believe mine is better :D, we'll see where everything pans out in the big scheme of things and how matt, bfarber and the rest of the ips crew see fit to go ahead.

[Sam A]

"If eval() is the answer, you're almost certainly asking the wrong question."