vb.org Archive
Page 1 of 2 1 2
Show 40 post(s) from this thread on one page

vb.org Archive (https://vborg.vbsupport.ru/index.php)
-   Advanced Warning System (AWS) (https://vborg.vbsupport.ru/forumdisplay.php?f=105)
-   -   Very Big Sql Injection Hole (https://vborg.vbsupport.ru/showthread.php?t=77743)

pimpery 03-08-2005 09:11 PM
Very Big Sql Injection Hole
 
aghhh! injection? in a premium modification? >.<
Example:
Warn.php?&do=ViewWarnings&id=1/

Input isnt escaped before being put into the sql query. Seriously, what the ****. A premium modification that doesn't even check the input :o

I made a Fix:
open warn.php

find:

Code:
// #######################################################################
// ######################## START MAIN SCRIPT ############################
// #######################################################################


Below that insert:

Code:
//SQL-safe modification
function safescape($key,&$value){
$value = mysql_escape_string($value);
}
$func = 'safescape';
array_walk(&$_GET,$func);
array_walk(&$_POST,$func);
//SQL safety mod done

sv1cec 03-09-2005 04:34 AM
Quote:
Originally Posted by pimpery
aghhh! injection? in a premium modification? >.<
Example:
Warn.php?&do=ViewWarnings&id=1/

Input isnt escaped before being put into the sql query. Seriously, what the ****. A premium modification that doesn't even check the input :o

I made a Fix:
open warn.php

find:

Code:
// #######################################################################
// ######################## START MAIN SCRIPT ############################
// #######################################################################


Below that insert:

Code:
//SQL-safe modification
function safescape($key,&$value){
$value = mysql_escape_string($value);
}
$func = 'safescape';
array_walk(&$_GET,$func);
array_walk(&$_POST,$func);
//SQL safety mod done
Could you please elaborate? I am not sure I follow you.

pimpery 03-10-2005 02:21 AM
Quote:
Originally Posted by sv1cec
Could you please elaborate? I am not sure I follow you.
what do u mean. therse an sql injection hole

look uptop at the example i posted.

sv1cec 03-10-2005 04:33 AM
Pimpery, not all of us have extreme experience in SQL or php or the works. From the example you show, I can assume two things :

1. The problems is that anyone can see the admin's warnings.
2. The slash at the end may allow hackers to insert code (for this I am not sure, but having spend some time reading articles I got containing the "SQL injection" phrase here, that's what I assume.

I am going to spend some more time today, figuring out this whole issue, but what I fail to understand is the function you provided.

So, please, instead of just posting a warning thread, saying VERY BIG HOLE WOLF, WOLF, you could spend some minutes helping me out understand how to close the hole and how to use that code. As I said, not all of us were born with that knowledge.

Rgds

Soup 03-10-2005 06:47 AM
In the Warning System (v3.1.9) in Warn.php, on line 332 you are running a potentially unsafe query (for example if a user enters a non-numerical input as the id variable). If you enter a string that doesn't start with a number (such as "foo") as the id the code will catch the problem on line 328-330, however if the string entered starts with a number (such as "3foo") then it will pass through the check fine.

In order to fix this:

Before line 328-330 add:
PHP Code:
$_GET['id'] = intval($_GET['id']); 
or globalize the variable:
PHP Code:
globalize($_GET,array('id' => INT)); 
and then you use $id instead of $_GET['id'].

sv1cec 03-10-2005 07:09 AM
Quote:
Originally Posted by Soup
In the Warning System (v3.1.9) in Warn.php, on line 332 you are running a potentially unsafe query (for example if a user enters a non-numerical input as the id variable). If you enter a string that doesn't start with a number (such as "foo") as the id the code will catch the problem on line 328-330, however if the string entered starts with a number (such as "3foo") then it will pass through the check fine.

In order to fix this:

Before line 328-330 add:
PHP Code:
$_GET['id'] = intval($_GET['id']); 
or globalize the variable:
PHP Code:
globalize($_GET,array('id' => INT)); 
and then you use $id instead of $_GET['id'].
Soup,

Thanks for the details, much appreciated. I have already been working on this, since this morning, and I have globalized all the _GET or _POST variables. But I was not aware that you can use kist $id instead of $_GET['id']

So any variable entered in the globalize function, can then be used with the part withing the brackets, that's what I figure out from looking at the globalize code. Am I correct? And how do you differentiate if you have a _GET['do'] and a _POST['do']?

Also, for STR variables, except from globalizing them (maybe using STR_NOHTML), is it also necessary to addslashes to them?

Again, thanks for the input, sincerely appreciated.

Soup 03-10-2005 08:47 AM
Quote:
Originally Posted by sv1cec
Soup,

Thanks for the details, much appreciated. I have already been working on this, since this morning, and I have globalized all the _GET or _POST variables. But I was not aware that you can use kist $id instead of $_GET['id']

So any variable entered in the globalize function, can then be used with the part withing the brackets, that's what I figure out from looking at the globalize code. Am I correct? And how do you differentiate if you have a _GET['do'] and a _POST['do']?
That is correct. It would be the same as writing
PHP Code:
$id = intval($_GET['id']); 
The only way to check if it is _POST or _GET would be to check per if statements which one of them is set, however I don't think it is ever necessary to do so.

If you only want input from _GET, you use $_GET as the first parameter of globalize(), if you only want input from _POST you use $_POST. If you want input from both and don't care which one is used, you can use $_REQUEST.

Quote:
Originally Posted by sv1cec
Also, for STR variables, except from globalizing them (maybe using STR_NOHTML), is it also necessary to addslashes to them?

Again, thanks for the input, sincerely appreciated.
Yes. STR basically leaves the input untouched (besides calling trim()), and STR_NOHTML converts all HTML tags and double quotes (") into 'safe' equivalents, however the 'dangerous' (in terms of SQL Injections) single quotes (') are untouched. So if you are running a query, you will need to run addslashes() on it.

sv1cec 03-10-2005 08:54 AM
Quote:
Originally Posted by Soup
That is correct. It would be the same as writing
PHP Code:
$id = intval($_GET['id']); 
The only way to check if it is _POST or _GET would be to check per if statements which one of them is set, however I don't think it is ever necessary to do so.

If you only want input from _GET, you use $_GET as the first parameter of globalize(), if you only want input from _POST you use $_POST. If you want input from both and don't care which one is used, you can use $_REQUEST.



Yes. STR basically leaves the input untouched (besides calling trim()), and STR_NOHTML converts all HTML tags and double quotes (") into 'safe' equivalents, however the 'dangerous' (in terms of SQL Injections) single quotes (') are untouched. So if you are running a query, you will need to run addslashes() on it.
One more question, if you do not mind. Can the globalize be located at the top of the file, right after requst or request_once, or shall it be right above the _GET in the various segments of the file?

Also, I tried echoing an _GET variable, before and after the globalize command, and indeed the contents are different. Can I still use the _GET['xxx'] after the globalize, or is it a must that I should use $xxx?


Tnx again

Soup 03-10-2005 10:08 AM
Quote:
Originally Posted by sv1cec
One more question, if you do not mind. Can the globalize be located at the top of the file, right after requst or request_once, or shall it be right above the _GET in the various segments of the file?

Also, I tried echoing an _GET variable, before and after the globalize command, and indeed the contents are different. Can I still use the _GET['xxx'] after the globalize, or is it a must that I should use $xxx?


Tnx again
Typically you're going to have a globalize() line inside each 'do' section, where it globalizes all the variables that you need for that 'do' section. You could globalize it all at the beginning (after including global.php) and everything will work just fine, but it is not 'clean' coding and will add a slight (negligible in my opinion) load for intializing variables don't exist otherwise (though that most likely will not have a noticeable effect, it's just the principle of it ;))

The first parameter of globalize is passed along as a reference, so the original variable is changed and you could continue to use $_GET['foo'] after globalizing it. However it is usually much less to write if you just use $foo instead and will make the code a bit more nicer looking.

sv1cec 03-10-2005 10:51 AM
Quote:
Originally Posted by Soup
Typically you're going to have a globalize() line inside each 'do' section, where it globalizes all the variables that you need for that 'do' section. You could globalize it all at the beginning (after including global.php) and everything will work just fine, but it is not 'clean' coding and will add a slight (negligible in my opinion) load for intializing variables don't exist otherwise (though that most likely will not have a noticeable effect, it's just the principle of it ;))

The first parameter of globalize is passed along as a reference, so the original variable is changed and you could continue to use $_GET['foo'] after globalizing it. However it is usually much less to write if you just use $foo instead and will make the code a bit more nicer looking.
Soup,

Many thanks for this quite interesting tuitorial. Really very very helpful.

I have applied your suggestions, and I'll release a new version of AWS in a while.

Kind regards and many thanks


All times are GMT. The time now is 08:31 AM.
Page 1 of 2 1 2
Show 40 post(s) from this thread on one page

Powered by vBulletin® Version 3.8.12 by vBS
Copyright ©2000 - 2025, vBulletin Solutions Inc.

X vBulletin 3.8.12 by vBS Debug Information
  • Page Generation 0.01232 seconds
  • Memory Usage 1,770KB
  • Queries Executed 10 (?)
More Information
Template Usage:
  • (1)ad_footer_end
  • (1)ad_footer_start
  • (1)ad_header_end
  • (1)ad_header_logo
  • (1)ad_navbar_below
  • (4)bbcode_code_printable
  • (6)bbcode_php_printable
  • (8)bbcode_quote_printable
  • (1)footer
  • (1)gobutton
  • (1)header
  • (1)headinclude
  • (6)option
  • (1)pagenav
  • (1)pagenav_curpage
  • (1)pagenav_pagelink
  • (1)post_thanks_navbar_search
  • (1)printthread
  • (10)printthreadbit
  • (1)spacer_close
  • (1)spacer_open 
Phrase Groups Available:
  • global
  • postbit
  • showthread
Included Files:
  • ./printthread.php
  • ./global.php
  • ./includes/init.php
  • ./includes/class_core.php
  • ./includes/config.php
  • ./includes/functions.php
  • ./includes/class_hook.php
  • ./includes/modsystem_functions.php
  • ./includes/class_bbcode_alt.php
  • ./includes/class_bbcode.php
  • ./includes/functions_bigthree.php 
Hooks Called:
  • init_startup
  • init_startup_session_setup_start
  • init_startup_session_setup_complete
  • cache_permissions
  • fetch_threadinfo_query
  • fetch_threadinfo
  • fetch_foruminfo
  • style_fetch
  • cache_templates
  • global_start
  • parse_templates
  • global_setup_complete
  • printthread_start
  • pagenav_page
  • pagenav_complete
  • bbcode_fetch_tags
  • bbcode_create
  • bbcode_parse_start
  • bbcode_parse_complete_precache
  • bbcode_parse_complete
  • printthread_post
  • printthread_complete