4
\$\begingroup\$

I am working with Youtube OAuth for the first time and I am not that great when it comes to security. I got the following code and would like to hear if you guys can find any security issue:

    $code = $_GET['code'];

    $url = 'https://accounts.google.com/o/oauth2/token';

    $params = array(
        "code" => $code,
        "client_id" => "XXX",
        "client_secret" => "YYY",
        "redirect_uri" => "URL",
        "grant_type" => "authorization_code"
    );

    $curl = curl_init($url);

    curl_setopt($curl, CURLOPT_POST, true);
    curl_setopt($curl, CURLOPT_POSTFIELDS, $params);
    curl_setopt($curl, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
    curl_setopt($curl, CURLOPT_SSL_VERIFYPEER, false);
    curl_setopt($curl, CURLOPT_RETURNTRANSFER, 1);  

    $json_response = curl_exec($curl);
    curl_close($curl);
    $authObj = json_decode($json_response);

    $accountInfo = get_data('https://www.googleapis.com/youtube/v3/channels?part=id&mine=true&access_token='.$authObj->access_token.'');

    $accountInfo = json_decode($accountInfo, true);

    $linkedAccount = $db->QueryFetchArrayAll("SELECT * FROM `youtube_accounts` WHERE `channel_id`='".$accountInfo['items'][0]['id']."'");
    $accountExists = $db->QueryFetchArrayAll("SELECT * FROM `youtube_accounts` WHERE `user_id`='".$data['id']."' AND `status` != 3");
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review. What does the code actually do? \$\endgroup\$ – Phrancis May 3 '15 at 0:44
2
\$\begingroup\$

Security

Curl Set POST

I don't think that there is a problem with passing user input to curl_setopt as post field (something like http header splitting is not possible, and it doesn't even seem possibly to add additional post fields, or to overwrite existing ones).

Important information via GET

It's not a good idea to send anything confidential via GET. GET requests can be cached, they can be stored in the browser history, they show up in server logs, and so on. Use POST instead.

SQL injection

$accountInfo isn't controlled by you, so you shouldn't trust it, the same probably goes for $data. Never put variable data directly into an SQL query, use prepared statements instead.

CURLOPT_SSL_VERIFYPEER

If you disable this setting, you weaken the security of SSL. Yes, you still encrypt even if CURLOPT_SSL_VERIFYPEER is false, but you would also accept the key of a man in the middle instead of the key from the server, so an attacker could still read the communication.

Here is the warning from the documentation:

WARNING: disabling verification of the certificate allows bad guys to man-in-the-middle the communication without you knowing it. Disabling verification makes the communication insecure. Just having encryption on a transfer is not enough as you cannot be sure that you are communicating with the correct end-point.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.