[BUG] In Billing Module.

[Obada8]

This bug can allow the hacker to edit value of max player or duration invoice to 0 and install the game for free.

• Fixed by this edit.

[own3mall]

Shouldn't the check be:

if($max_players < 1 || $qty < 1){

Why are you limiting it to less than 10? Can't you pick between 1-64 slots in most cases?

[Obada8]

Sorry, but i limit it to 10 because my max player must be at least 10 in store (Read the price formula) $price = $_POST['max_players'] * $price_slot * $_POST['qty']; I think we can replace it (at least slots) by $row['slot_min_qty']; Done. if(isset($_REQUEST['service_id'])) $where_service_id = " WHERE service_id=".$_REQUEST['service_id']; else $where_service_id = ""; $qry_services = "SELECT * FROM OGP_DB_PREFIXbilling_services".$where_service_id; $services = $db->resultQuery($qry_services); foreach ($services as $key => $row) { if($max_players < $row['slot_min_qty'] || $qty < 1){ $max_players = $row['slot_min_qty']; $qty = 1; } }

[own3mall]

I still prefer my check. Can you make it be the code snippet I posted originally?

[Obada8]

What you mean? Btw, Consider this pull as a notice

[own3mall]

Instead of making another database call:

https://github.com/OpenGamePanel/Module-Simple-billing/pull/3/commits/1aef4d5ffbb37a5a260d12ec805bd4ca28b6ceb3

Use for the check:

if($max_players < 1 || $qty < 1){ // Set defaults below.

No biggie. I guess I'll just do it.

[Obada8]

Then close my pull? If yes, Please make your change then when i back here again i will close my pull. *If you use this max_players < 1 then if i hacker i can play with price from 1 slots to maximum slots but if you use. if($max_players < $row['slot_min_qty'] Then i can't set slots to 1 if my setting is 10 as minimum slots.

This for duration as example: Screenshot.

[own3mall]

That's not really hacking. If they get a price that is incorrect, you'll simply delete their server.

[Adjokip]

I think checking the database for the slot count allowed would be good.

If this commit is going to be accepted with the extra database call, then I think the following should be done too:

1) Add an extra check added for the inverse: check max_players against slot_max_qty. It would be good to do in the event someone is only selling a max of 16 slots per server, but someone edits the option value.

2) Cast $_REQUEST['service_id'] to an int and then check if its value is higher than 0 before using it in the WHERE clause. Checking if it's higher than 0 because if it's a non-numeric value, after casting it to an int it'll be 0.

3) The service id around line 32 - 35 should also be cast to an int, or checked if it's numeric before used in the WHERE clause... otherwise an SQL error is possible currently. If it's not an int (or if it's 0 after casting) redirect to the shop page.

4) The require config line can be removed... it is already included by the point the module is executed and it only contains the database connection info, and the database is accessed via the $db global. I'm guessing that was left over from when the exec_ogp_module function didn't exist.

[own3mall]

If you could do what adjo says @Obada8, you'd be saving me time!

[Obada8]

@own3mall I am sorry i am still noob in php, i just learn the basics, cause i have exames in these days, I make update, But it still need to check if id of the service is exist or not because we still can change it to any number is higher than 0 with number not attached to service.

[Adjokip]

Maybe also consider the following:

1) Line 30, 31 can be removed as I said above. A connection is made before the module is executed.

2) Change global $db to global $db, $view otherwise when $view is called a fatal error will be produced... this also means line 115: global $view can be removed as it's redundant.

3) Change if(isset($service_id)) to if($service_id !== 0) - the reason being, $service_id is always going to be set now. You'll want to add the WHERE clause if it's not 0.

4) To ensure it's a valid service id, you'll want to check what's returned from $result_service like so:

    if ($result_service === false) {
        $view->refresh('home.php?m=simple-billing&p=shop');
    }

The following should also be changed in shop.php

1) Change global $db to global $db, $view, like above.

2) Get the intval of $_REQUEST['service_id'] and do the same conditional check on it not being 0. If it's not zero, add the WHERE clause on line 64,

3) To check it's a valid service id, the following should be added after $services - like above:

    if (isset($_REQUEST['service_id']) && $services === false) {
        $view->refresh('home.php?m=simple-billing&p=shop');
    }

[Obada8]

Make new update, But i still have problem when i delete require('includes/config.inc.php');. The error is Table 'xxx_panel.billing_services' doesn't exist So i kept it.

[Adjokip]

That's happening because $table_prefix is used in the query, but the database class searches the SQL for OGP_DB_PREFIX ... so, what that means is you can remove the include/require line, and change line 38 to:

$qry_service = "SELECT DISTINCT service_id, home_cfg_id, mod_cfg_id, service_name, remote_server_id, slot_max_qty, slot_min_qty, price_hourly, price_monthly, price_year, description, img_url FROM OGP_DB_PREFIXbilling_services WHERE service_id=".$service_id;

But please do step 3. for shop.php... otherwise the hidden elements of each game image can be changed, and when clicked a blank page is shown (actually, warnings with display_errors on - since false is being passed to foreach)

So after line 66/67 in shop.php, check if $services is false and $_REQUEST['service_id'] is set, if that condition is true, redirect - like I posted above.

[Obada8]

Yes it's work now without line require('includes/config.inc.php');. Thank you :) And for step 3 i forget it sorry. Now it's committed. I think it's done now, Right?

[Adjokip]

Yes. Everything I've noted down is done now... and the service id, along with slot count, etc should always be valid in those two files.

It'll be up to @own3mall to accept the requests after he's looked over them, or give any input and/or suggestions.