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
  • 1
    \$\begingroup\$ Welcome to Code Review. What does the code actually do? \$\endgroup\$
    – Phrancis
    May 3, 2015 at 0:44

1 Answer 1

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 and acknowledge that you have read and understand our privacy policy and code of conduct.

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