Are there any vulnerabilities in this code?

if (isset($_COOKIE['lang'])) { $language = $_COOKIE['lang']; $result = "en"; if (in_array($language, $LANGS)) { $result = $language; } @setcookie("lang", $result, time() + 14 * 24 * 3600, "/"); include_once("../sys/lang/".$result.".php"); } else { $language = "en"; if (isset($_SERVER['HTTP_ACCEPT_LANGUAGE'])) { $language = substr($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2); } $result = "en"; if (in_array($language, $LANGS)) { $result = $language; } @setcookie("lang", $result, time() + 14 * 24 * 3600, "/"); include_once("../sys/lang/".$result.".php"); }
  • Are you only interested in security? Does it bother you that the code completely duplicates the same actions in vain? - AK ♦

3 answers 3

Check for a cookie not for existence, but for emptiness. Plus the size of your code can be reduced.

Ready option:

 $result = 'en'; if(!empty($_COOKIE['lang']) && in_array($_COOKIE['lang'], $LANGS)) { $result = $_COOKIE['lang']; }else if(!empty($_SERVER['HTTP_ACCEPT_LANGUAGE'])) { $language = substr($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2); if (in_array($language, $LANGS)) $result = $language; } setcookie('lang', $result, time() + 14 * 24 * 3600, '/'); include_once('../sys/lang/'.$result.'.php');
  • undefined index. - Naumov
  • @Total Pusher, thank you for the example, although it is not clear how it relates to the code discussed above? The script is checked for the existence of a variable and can only be accessed if it is set - skarb
  • Yes you are right. For some reason I thought that empty when accessing a non-existent hash is issued by vorning. Does not issue. - Total Pusher

It is not clear what you would like to hear, but of course it is not safe, simple kais hacking.

  1. Writing a test.php script
  2. Applaud to the site through the form
  3. Find coockie lang = '.. / .. / .. / uploads / test.php'
  4. Success!

Content test.php

 <?php echo 'test';
  • one
    shield? 1) Where did you find the form? 2) How are you going to look for something? 3) If you can upload a pkhp script through the form, it’s not a matter of this piece but in the form. - Manitikyl
  • @Manitikyl Shyra think you give the opportunity to execute any script on the server through include, and this is already a hole. - Naumov
  • @Naumov why anyone? There after all there is an in_array check. Those. Only those scripts that the developer spelled out in the $ LANGS - skarb array can be included
  • @skarb in half a year will remove the check and everything that would automatically turn on includit' — Naumov
  • @Naumov ha)) Would have recognized their mistake, instead of embarrassing. Following your logic, for reliable security the site should be transferred to your home computer and disconnected from the Internet - skarb

For PHP7 +, you can use the join operator with null ?? .

It will be like this:

 $result = $_COOKIE['lang'] ?? ( isset($_SERVER['HTTP_ACCEPT_LANGUAGE']) ? substr($_SERVER['HTTP_ACCEPT_LANGUAGE'], 0, 2) : 'en' ); if (!in_array($result, $LANGS)) $result = 'en'; setcookie('lang', $result, time() + 14 * 24 * 3600, '/'); include_once('../sys/lang/'.$result.'.php');
  • one
    Coding stayl nervously smokes, and developed php7 + prick the eyes - Naumov
  • @Naumov what specifically do you dislike in this example in order to understand your mistake? The complex structure is divided into 2 through the takeaway on a separate line. - Total Pusher