Security Bug in My CMS?

sw33t_r3v3ng3

I was testing out my CMS i built earlier and i discovered a bug which allows a form open to xss. Can someone help me to fix it, not quite sure where I went wrong!

Code for editArticle Function :

function editArticle() {
 
  $results = array();
  $results['pageTitle'] = "Edit Article";
  $results['formAction'] = "editArticle";
 
  if ( isset( $_POST['saveChanges'] ) ) {
 
    // User has posted the article edit form: save the article changes
 
    if ( !$article = Article::getById( (int)$_POST['articleId'] ) ) {
      header( "Location: admin.php?error=articleNotFound" );
      return;
    }
 
    $article->storeFormValues( $_POST );
    $article->update();
    header( "Location: admin.php?status=changesSaved" );
 
  } elseif ( isset( $_POST['cancel'] ) ) {
 
    // User has cancelled their edits: return to the article list
    header( "Location: admin.php" );
  } else {
 
    // User has not posted the article edit form yet: display the form
    $results['article'] = Article::getById( (int)$_GET['articleId'] );
    require( TEMPLATE_PATH . "/admin/editArticle.php" );
  }
 
}

ChRoMe

sw33t_r3v3ng3 wrote: »

I was testing out my CMS i built earlier and i discovered a bug which allows a form open to xss. Can someone help me to fix it, not quite sure where I went wrong!

Code for editArticle Function :

function editArticle() {
 
  $results = array();
  $results['pageTitle'] = "Edit Article";
  $results['formAction'] = "editArticle";
 
  if ( isset( $_POST['saveChanges'] ) ) {
 
    // User has posted the article edit form: save the article changes
 
    if ( !$article = Article::getById( (int)$_POST['articleId'] ) ) {
      header( "Location: admin.php?error=articleNotFound" );
      return;
    }
 
    $article->storeFormValues( $_POST );
    $article->update();
    header( "Location: admin.php?status=changesSaved" );
 
  } elseif ( isset( $_POST['cancel'] ) ) {
 
    // User has cancelled their edits: return to the article list
    header( "Location: admin.php" );
  } else {
 
    // User has not posted the article edit form yet: display the form
    $results['article'] = Article::getById( (int)$_GET['articleId'] );
    require( TEMPLATE_PATH . "/admin/editArticle.php" );
  }
 
}

No helpful answer: Annnnnnnnnnnnnnnd this is why you shouldn't build your own cms

Slightly helpful answer: Do a regex replace (or similar) encoding dangerous symbols

Colonel Panic

At least you didn't post *that* link.

Seriously though, there's nothing wrong with wanting to make your own CMS and learn how to guard against XSS attacks, is there? What happens when the last people who actually understand this die and all that's left is frameworks?

The problem with the OP's post is the same as all the OP's posts. He doesn't show what he's tried. This stuff is all a Google away, but it takes more than copy pasting tutorial code and changing it until there's no errors.

sw33t_r3v3ng3

Colonel Panic wrote: »

At least you didn't post *that* link.

Seriously though, there's nothing wrong with wanting to make your own CMS and learn how to guard against XSS attacks, is there? What happens when the last people who actually understand this die and all that's left is frameworks?

The problem with the OP's post is the same as all the OP's posts. He doesn't show what he's tried. This stuff is all a Google away, but it takes more than copy pasting tutorial code and changing it until there's no errors.

I much prefer to build my own projects because firstly it gives me experience and i learned a hell of a lot more on this then I already knew, and secondly because i just couldn't be bothered messing around with frameworks like cake php and making them work with my system.

What do you mean by I dont show what ive done? How can I improve on this and make the question more helpful??

Talisman

You could start by explaining what security you have put in place and what you see as the issue.

My 2 second observation : You've got no security.

$article->storeFormValues( $_POST );
$article->update();
header( "Location: admin.php?status=changesSaved" );

Always validate that a user role has sufficient privileges for the task they are attempting to perform i.e. to create, read, update, or delete data.

Once you've established the level of privilege is sufficient to write to the database, scrub the data before saving it.

Have a read through the OWASP Cheat Sheet to help you identify potential security risks.

Gotham

I'm only taking a guess here, but I think the problem is in this function.
$article->storeFormValues( $_POST );

Inside that function make sure that your values are html escaped.
If the data is being saved in sql as well (which it should) you should probably use prepared statements and also SQL escape them.

But it's likely that your xss is coming from not using: http://www.w3schools.com/php/func_string_htmlspecialchars.asp

And this will not be your only problem as you continue to develop your software.

azzeretti

ChRoMe wrote: »

No helpful answer: Annnnnnnnnnnnnnnd this is why you shouldn't build your own cms

I'm sure you don't really mean this, do you? Is this just CMS software that the world has a perfectly perfect supply of or all software? Any other software we shouldn't try to develop alone and make better?

Luckily we don't need any SSL/TLS libraries now that OpenSSL is rock sold and bug free. Oh, wait......

ChRoMe

azzeretti wrote:

I'm sure you don't really mean this, do you? Is this just CMS software that the world has a perfectly perfect supply of or all software? Any other software we shouldn't try to develop alone and make better?

CMS is a solved problem, grand for a learning exercise but it's a waste of time writing your own for the vast majority of use cases.

ChRoMe

It's classic "not invented here syndrome" a common phrase in this industry.

Colonel Panic

But the guy is learning how to program, not working in the industry.

ChRoMe

Colonel Panic wrote:

But the guy is learning how to program, not working in the industry.

That's why I prefixed the statement as not helpful answer.

azzeretti

ChRoMe wrote: »

CMS is a solved problem.

"I think there is a world market for maybe five computers" - chairman of IBM, 1943.

Uninstall your IDE, "rm ./.git -r" , take up knitting. Software is "solved" !!

sw33t_r3v3ng3

Talisman wrote: »

You could start by explaining what security you have put in place and what you see as the issue.

My 2 second observation : You've got no security.



Always validate that a user role has sufficient privileges for the task they are attempting to perform i.e. to create, read, update, or delete data.

Once you've established the level of privilege is sufficient to write to the database, scrub the data before saving it.

Have a read through the OWASP Cheat Sheet to help you identify potential security risks.

Thanks for your reply. I see what you mean here but ive tried the exact same xss attact (<script>alert(document.cookie);</script>) on all other forms but its just one form the code executes on?

ChRoMe

azzeretti wrote:

Uninstall your IDE, "rm ./.git -r" , take up knitting. Software is "solved" !!

No, just CMS.

azzeretti

ChRoMe wrote: »

No, just CMS.

If CMS, or indeed any application, was solved, then there wouldn't be new repo's popping up on github regularly. Nor, would Drupal 8 be about to drop. After all, if it was "solved" then there wouldn't be any need for maintenance. There would also be no possibility of any feature enhancements.

CMS is a huge market and is constantly evolving. To say it's solved is myopic at best. Sure, wasn't SCM "solved" with Bitkeeper? Linus wasn't so sure.
BitTorrentSync "solved" all our file syncing problems? Minix was the end of propriatery Unix etc, etc, etc. Sure Microsoft have the "perfect" OS now!!

Sorry to the OP as this isn't really helping with the initial post. I suppose my point is that you shouldn't stop trying to learn. If you think writting your own app will aid your development then go for it. Obviously there may be security issues, but why not use those issues to keep learning instead of relying on the ready made "solved" solution that may be 95% bloat and/or closed source (where you've no idea what is going on!)

sw33t_r3v3ng3

azzeretti wrote: »

If CMS, or indeed any application, was solved, then there wouldn't be new repo's popping up on github regularly. Nor, would Drupal 8 be about to drop. After all, if it was "solved" then there wouldn't be any need for maintenance. There would also be no possibility of any feature enhancements.

CMS is a huge market and is constantly evolving. To say it's solved is myopic at best. Sure, wasn't SCM "solved" with Bitkeeper? Linus wasn't so sure.
BitTorrentSync "solved" all our file syncing problems? Minix was the end of propriatery Unix etc, etc, etc. Sure Microsoft have the "perfect" OS now!!

Sorry to the OP as this isn't really helping with the initial post. I suppose my point is that you shouldn't stop trying to learn. If you think writting your own app will aid your development then go for it. Obviously there may be security issues, but why not use those issues to keep learning instead of relying on the ready made "solved" solution that may be 95% bloat and/or closed source (where you've no idea what is going on!)

Thanks for the reply man! I agree with what you said above about using it co continue learning! However, Microsof having the perfect OS? Please :') My laptops BIOS is more stable than Windows 8 :')

croo

What bugs me is that the OP copies code line for line from someone else and presents it as his own. Something I think Colonel Panic alludes to OP when he said you didn't show what you tried - you showed what someone else tried!

This is called plagiarism OP and it's a bad habit to get into at your young age. There is no harm copying code - just make sure it's clear that you did.

In defense of of Chrome, too many people insist on building things that are already available is the FOSS world... they'd be better off starting with it and if need be improving it, rather than building their own version that adds nothing other than restricts their customers and makes it difficult for them to move on (if they want).

Not that someone cannot create a better CMS but suppose we look at Drupal since it was mentioned already... according to Ohloh (now called open hub), there are 299 man years of effort in creating it. What small design house can make that sort of commitment? Not many I would venture. I suspect many home grown CMS (and lot of other apps too) are more about protecting the seller's market than what's in the best interest of the customer.

ChRoMe

That's what I was getting at but was just too lazy posting detail as I'm on the phone, thanks.

Colonel Panic

Do you ALWAYS post on your phone, then?

ChRoMe

Colonel Panic wrote:

Do you ALWAYS post on your phone, then?

pretty much for the last month

sw33t_r3v3ng3

croo wrote: »

What bugs me is that the OP copies code line for line from someone else and presents it as his own. Something I think Colonel Panic alludes to OP when he said you didn't show what you tried - you showed what someone else tried!

This is called plagiarism OP and it's a bad habit to get into at your young age. There is no harm copying code - just make sure it's clear that you did.

In defense of of Chrome, too many people insist on building things that are already available is the FOSS world... they'd be better off starting with it and if need be improving it, rather than building their own version that adds nothing other than restricts their customers and makes it difficult for them to move on (if they want).

Not that someone cannot create a better CMS but suppose we look at Drupal since it was mentioned already... according to Ohloh (now called open hub), there are 299 man years of effort in creating it. What small design house can make that sort of commitment? Not many I would venture. I suspect many home grown CMS (and lot of other apps too) are more about protecting the seller's market than what's in the best interest of the customer.

Just to make things clear, I dont know enough PHP to go about building a CMS with my little knowledge. I did however google "CMS code example" and did NOT copy and paste. I did copy it line for line but just to understand howit works. The code in the first post isn't mine. It was copied from an example! I stand to be corrected but dont think i said anywhere it was my own code?

Colonel Panic

You called it "my CMS".

sw33t_r3v3ng3

Colonel Panic wrote: »

You called it "my CMS".

Ok well i apologise if i mislead people!

Talisman

sw33t_r3v3ng3 wrote: »

Thanks for your reply. I see what you mean here but ive tried the exact same xss attact (<script>alert(document.cookie);</script>) on all other forms but its just one form the code executes on?

Why do you think that might be?

sw33t_r3v3ng3

Talisman wrote: »

Why do you think that might be?

I have no clue!

Talisman

XSS vulnerabilities occur when input is not validated and output is not verified.

I showed you the source of the issue in my original response. Your code does not check the content of the $_POST variable, instead it is passed directly to the storeFormValues() function.

sw33t_r3v3ng3

Talisman wrote: »

XSS vulnerabilities occur when input is not validated and output is not verified.

I showed you the source of the issue in my original response. Your code does not check the content of the $_POST variable, instead it is passed directly to the storeFormValues() function.

Ah okay! I get ya, so is it htmlspecischars i need to add in?

croo

Some words from the original article "Build a CMS" that might help.

The title and summary are filtered using a regular expression to only allow a certain range of characters. It's good security practice to filter data on input like this, only allowing acceptable values and characters through.

We don't filter the content property, however. Why? Well, the administrator will probably want to use a wide range of characters, as well as HTML markup, in the article content. If we restricted the range of allowed characters in the content then we would limit the usefulness of the CMS for the administrator.

Normally this could be a security loophole, since a user could insert malicious JavaScript and other nasty stuff in the content. However, since we presumably trust our site administrator — who is the only person allowed to create the content — this is an acceptable tradeoff in this case. If you were dealing with user-generated content, such as comments or forum posts, then you would want to be more careful, and only allow "safe" HTML to be used.

And the author goes on to recommend a tool that might be used if we did want to filter the input.

A really great tool for this is HTML Purifier, which thoroughly analyses HTML input and removes all potentially malicious code.

Finally the author offers some further security related reading

PHP security is a big topic, and beyond the scope of this tutorial. If you'd like to find out more then start with Terry Chay's excellent post, Filter Input-Escape Output: Security Principle and Practice. Also see the Wikipedia entries on secure input/output handling, XSS, CSRF, SQL injection, and session fixation.

sw33t_r3v3ng3

croo wrote: »

Some words from the original article "Build a CMS" that might help.


And the author goes on to recommend a tool that might be used if we did want to filter the input.

Finally the author offers some further security related reading

Thanks alot! Will have a read through this!