my registration Proccess is HACKED! (how?) (L5.1 version)

was the account verified?

whats your rules like?

i dont see anything suspicious about that entry..

As I already answered @shez1983 the rules are:

        private function rules()
        {
            $recaptcha = [
                'g-recaptcha-response'  => 'required|checkcaptcha'
            ];

            $rules = [
                'username'             => 'required|min:3|max:15|unique:users,username',
                'email'                 => 'required|email|unique:users,email',
                'password'              => 'required|confirmed|regex:/^[a-zA-Z0-9].{7,15}$/',
                'terms'                 => 'required'
            ];

            if(\Config::get('recaptcha.enabled')){
                return $rules + $recaptcha;
            }else{
                return $rules;
            }
        }

@shez1983 the entry has:

• no username (required)
• no password (required)
• no token (auto created)

@Snapey the account did not go to users table. There was only in registration table. The registration is the middle table helping for verification. Account did not verified...

you validate the input then

$new_user = Register::firstOrCreate(['email' => $request->get('email')]);

create an entry in Register model

but dont write any of the other values

if the record is new you then abort 403

        if($new_user->exists()){
            $new_user->update($this->registerFields($request,$token));
        }else{
            abort(403);
            //$new_user->create($this->registerFields($request, $token));
        }

unless I am reading this wrong your registration is broken ?

yes @snapey, my registration is broken.

It seems the problem is between that code:

1.            $new_user = Register::firstOrCreate(['email' => $request->get('email')]);
2.
3.            $token = Str::random(128);
4.
5.            if($new_user->exists()){
                $new_user->update($this->registerFields($request,$token));
            }else{
                abort(403);
                //$new_user->create($this->registerFields($request, $token));
            }

For some reason the process is broken between the lines 1 & 3. If there was a transcation I suppose the problem would be solved.

But why the code is broken between the lines?

no problem there.

i dont understand why you abort if the user has not already registered

@snapey because Register::firstOrCreate is always creates a record or find it. So always exists. In other case must abort

@nicsmyrn The logic is hard to follow in your controller. I don’t know why you’re creating this “Register” model—why don’t you just add a activated column to your users table?

@martinbean I dont want unregistered users to mess my users table. I want to isolate registered users (in users table) and those who want to register in registration table. Second I dont want to have ghost records in my users table.

There is no reason to have all information to one table.

Can you actually register?

@snapey with this script, 500 users are already registered...

As I already told it seems that the code is broken between the lines 1 and 6:

1.            $new_user = Register::firstOrCreate(['email' => $request->get('email')]);
2.
3.            $token = Str::random(128);
4.
5.            if($new_user->exists()){
6.                $new_user->update($this->registerFields($request,$token));
7.           }else{
8.                abort(403);
9.                //$new_user->create($this->registerFields($request, $token));
10.            }

The code in line 1. inserted a record in database with only email field and then the code in line 6 never runned

I dont want unregistered users to mess my users table

@nicsmyrn Yeah. You’re just messing up your registration logic, instead.

I want to isolate registered users

SELECT * FROM users WHERE verified = 1

Just sounds like your standard SQL injection attack. Have you checked the DB access privileges? Laravel protects pretty well against an SQL attack however, the actual configuration of the DB server is another story. This is why it is always a good practice to validate the data on the client and clean and validate on the server side before insertion. Most of these things will be caught before insertion. You are just lucky that all they could do was part of the registration record.

@ejdelmonico its the first good answer until now. Unfortunately I use the root mysql user with full privileges (I know its bad thing). Do you suggest mysql user (for laravel app) to have only database table privileges like select, insert, update, delete or I need extra privileges? For my laravel app one solution I found is to merge the two sql requests in to one: The old code used firstOrCreate and then update (two sql requests). The code below i believe uses only select query (for searching) and insert (no update)

            $new_user = Register::firstOrNew(['email' => $request->get('email')]);

            $new_user->token = Str::random(128);
            $new_user->password = bcrypt($request->get('password'));
            $new_user->email = $request->get('email');
            $new_user->username = $request->get('username');
            $new_user->save();

What I don't understand is how you're able to insert a record into your registration table without setting the non-nullable fields. How are you not getting SQL errors every time you try to insert a registration with just the email when username and password are defined as NOT NULL without a default value?

Does "return $rules + $recaptcha;" really give the expected result?

Very good question @36864. The registration table NOT allows username, email, password and token to be null. But if I try to run something like this in a sql query it accepts it.

INSERT INTO `registration` (`id`, `username`, `email`, `password`, `token`, `created_at`, `updated_at`)
VALUES
    (148, '', '[email protected]', '', '', '2017-12-27 13:31:04', '2017-12-27 13:31:04');

So the suspicious user somehow made a query with zero string values "", not null values.

@Procat $rules + $recaptcha works fine. I use in my .env file a boolean value so I can enable or disable recaptcha in my local computer. Repatcha does not validate localhost.

            if(\Config::get('recaptcha.enabled')){
                return $rules + $recaptcha;
            }else{
                return $rules;
            }

$token = Str::random(128);

$new_user = Register::firstOrCreate(['email' => $request->get('email')], $this->registerFields($request, $token));
       

i don't get it why do you want to update the user model if user already registered? Just let the validator handlers it.

@rolemodelboyz this might have been the solution but unfortunately in Laravel 5.1 second parameter in firstOrCreate method not allowed. Laravel Api:: firstOrCreate

@nicysm The problem isn't even there, it's when you call firstOrCreate with only one field. How is that not causing a SQL exception?

@36864 I dont know. So far many people successfully registered. If I try this code to tinker causes a SQL exception:

Illuminate\Database\QueryException with message 'SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '' for key 'registration_username_unique' (SQL: insert into `registration` (`email`, `updated_at`, `created_at`) values ([email protected], 2017-12-29 15:15:57, 2017-12-29 15:15:57))'

Your error is on the line:

$new_user = Register::firstOrCreate(['email' => $request->get('email')]);

I think you want

$new_user = Register::firstOrNew(['email' => $request->get('email')]);

The first one actually creates a row in the Registers table with just an email. The second one will onlny create a model, without saving to the DB.

@lostdreamer_nl I have changed the code to:

            $new_user = Register::firstOrNew(['email' => $request->get('email')]);

            $new_user->token = Str::random(128);
            $new_user->password = bcrypt($request->get('password'));
            $new_user->email = $request->get('email');
            $new_user->username = $request->get('username');
            $new_user->save();

but I still don't know how this table-record passed to the database: [img]https://i.imgur.com/cBzvrcd.png[/img]

Looking at your code, I think there was an error in the part

$new_user->update($this->registerFields($request,$token));

Check your log around 27-12 13:31 for an SQL error. If there is, that's where it went wrong.

First it created an empty row with only an email field Then it tried to update that row, but failed for some reason, leaving the row with only an email.

@nicsmyrn The best way to set privileges in mysql or mariadb is to create a new user right after installing. You will need to use the root user to create the new account. Forge does this at server creation for you. So, create a user (call it what you want) like www and set the password, add dbuser privileges, save it and flush the privileges for it to work. From then on, you only use the new user to access the DB. At least it solves invaders from getting root access. Additionally, it works the same way on your server. Create the server and use the root user to sign in. Update the server and then create a new user without sudo privileges. Now do everything with the new user. If you need to install something or change a config, you will have to type su to change to root. Again forge does all of this for you for minimal cost. The system creates a forge user for the db and the server just like I am recommending.

@ejdelmonico thanks for your advices. For server my app and web server is under a simple user without sudo privileges. But I dont do the same for the database. I am going to follow your advise for db too. Thanks for the reply