5 ms·

It's not how much PHP is running, it's that PHP is running _at all_. It makes me feel very, very uneasy that every time that web interface is hit PHP executes a

by jdiez17 13y ago

It's not how much PHP is running, it's that PHP is running _at all_. It makes me feel very, very uneasy that every time that web interface is hit PHP executes a shell command. There's something inherently wrong about that, to me.

deanclatworthy 13y ago
Unless the script accepts parameters, which it doesn't, there's nothing to worry about.
- jdiez17 13y ago
  Like I said in a previous reply, see register_globals and company. Sure, that was some time ago. Sure, the defaults are better now. But at some point in time the people in charge of PHP thought "yes, this is a good idea, let's do it". ... It's not like the attitude has changed, though. There are many, many things deeply wrong with PHP when it comes to security. PHP is supposed to cater to unexperienced programmers. An unexperienced programmer might see "mysql_escape_string" and think that it will escape strings, making them suitable for use in SQL queries. The programmer will think the code is secure. WRONG. Because you have to use mysql_REAL_escape_string. Also, look at the `e` flag in preg_replace. WHAT THE FUCK. Like, seriously. What. Why. There are no words to describe how gobsmacked I am. And FOUR people in the PHP committee (or whatever it's called) voted __AGAINST__ deprecating it. FOUR. [1] -- The point is that I can't audit (and would rather not waste my time doing so) this PHP code. The fact that it uses shell_execute when a HTTP request demands it is enough of a red flag. [1] https://wiki.php.net/rfc/remove_preg_replace_eval_modifier https://wiki.php.net/rfc/remove_preg_replace_eval_modifier
  - SnacksOnAPlane 13y ago
    But you can totally audit the PHP code. There honestly isn't that much of it, and absolutely none of it takes user input. I'd be way more concerned if this was a Rails or Django app, because then there would be lots of library code to worry about.
    - anglebracket 13y ago
      It doesn't take input from the user, but it does use untrusted input in a way that allows XSS. See https://news.ycombinator.com/item?id=7128442 https://news.ycombinator.com/item?id=7128442 .
  - anglebracket 13y ago
    > WRONG. Because you have to use mysql_REAL_escape_string. Using mysql_real_escape_string is almost a sign you're doing something wrong. You should be using prepared statements with PDO or mysqli. > The point is that I can't audit (and would rather not waste my time doing so) this PHP code. I wasn't going to bother, but this post is pretty high up on the front page. There's some XSS issues with the JSON output, the Content-Type header isn't set to 'application/json' so PHP decides to set it to 'text/html'. Now anyone that controls ipecho.net[0] or can execute commands as any user on the server[1] can XSS users of the panel. If you'd like to confirm, go to /sh/ps.php and notice where the page breaks due to strings in the JSON being interpreted as HTML. [0] https://github.com/afaqurk/linux-dash/blob/master/sh/ip.php#L6 https://github.com/afaqurk/linux-dash/blob/master/sh/ip.php#... [1] https://github.com/afaqurk/linux-dash/blob/master/sh/ps.php#L4 https://github.com/afaqurk/linux-dash/blob/master/sh/ps.php#...
    - deanclatworthy 13y ago
      But the XSS issues have nothing to do with the language choice. Python, ruby and any other langauge do nothing by default to protect you against such things either. I agree this is a poor choice of code, and an attack vector, but the language used here is not to blame.
      - anglebracket 13y ago
        I was responding to the parent's unwillingness to audit the code, not so much about technical issues with PHP. The biggest security issue that I've noticed with PHP is more cultural: Developers are far more likely to write ad-hoc pages with subtle security issues than use well-tested frameworks and libraries because it seems easier. I would never expect to see Ruby or Python code that generates a JSON array like this [0], but I'm not at all surprised when I see it in PHP. It's too easy and tempting to do the wrong thing. [0]: https://github.com/afaqurk/linux-dash/blob/master/sh/users.php#L5 https://github.com/afaqurk/linux-dash/blob/master/sh/users.p...
        jdiez17 13y ago
  - deanclatworthy 13y ago
    What you are talking about are core language decisions. Good developers have known how to work around register globals for years now, and it's not been a problem in any modern web app I've touched for the last 8 years (off the top of my head). The language being used here has absolutely nothing to do with the output. You could substitute the PHP shell_exec calls for something similar in python, ruby or any other language. You should audit anything that hits your server. Using PHP doesn't meant it's more susceptible to vulnerabilities.
    - jdiez17 13y ago
      >Good developers have known how to work around register globals for years now, and it's not been a problem in any modern web app I've touched for the last 8 years (off the top of my head). A good programming language lets a good developer to focus on writing code, not wrestling with bullshit decisions. YES, good programmers can write good and safe PHP code. But in the same way, bad or unexperienced programmers WILL and DO write awful code. The most recent example is OP's code. I've clicked on a few links that people replying to me have posted, and honestly, I'm glad I didn't spend any more time looking at it. >The language being used here has absolutely nothing to do with the output. You could substitute the PHP shell_exec calls for something similar in python, ruby or any other language. Like I said in previous comments, substituting PHP by Python or Ruby wouldn't improve the situation very much; I agree with that. The correct way to do it is to separate the data collection from the web interface. I think that should be common sense, and I fear that PHP is eroding that by making it very easy to do it "the lazy way" (fuck it, we'll use shell_exec on the web, what's the worst that could happen right?).

by jdiez17 13y ago

deanclatworthy 13y ago

Unless the script accepts parameters, which it doesn't, there's nothing to worry about.

jdiez17 13y ago
Like I said in a previous reply, see register_globals and company. Sure, that was some time ago. Sure, the defaults are better now. But at some point in time the people in charge of PHP thought "yes, this is a good idea, let's do it". ... It's not like the attitude has changed, though. There are many, many things deeply wrong with PHP when it comes to security. PHP is supposed to cater to unexperienced programmers. An unexperienced programmer might see "mysql_escape_string" and think that it will escape strings, making them suitable for use in SQL queries. The programmer will think the code is secure. WRONG. Because you have to use mysql_REAL_escape_string. Also, look at the `e` flag in preg_replace. WHAT THE FUCK. Like, seriously. What. Why. There are no words to describe how gobsmacked I am. And FOUR people in the PHP committee (or whatever it's called) voted __AGAINST__ deprecating it. FOUR. [1] -- The point is that I can't audit (and would rather not waste my time doing so) this PHP code. The fact that it uses shell_execute when a HTTP request demands it is enough of a red flag. [1] https://wiki.php.net/rfc/remove_preg_replace_eval_modifier https://wiki.php.net/rfc/remove_preg_replace_eval_modifier
- SnacksOnAPlane 13y ago
  But you can totally audit the PHP code. There honestly isn't that much of it, and absolutely none of it takes user input. I'd be way more concerned if this was a Rails or Django app, because then there would be lots of library code to worry about.
  - anglebracket 13y ago
    It doesn't take input from the user, but it does use untrusted input in a way that allows XSS. See https://news.ycombinator.com/item?id=7128442 https://news.ycombinator.com/item?id=7128442 .
- anglebracket 13y ago
  > WRONG. Because you have to use mysql_REAL_escape_string. Using mysql_real_escape_string is almost a sign you're doing something wrong. You should be using prepared statements with PDO or mysqli. > The point is that I can't audit (and would rather not waste my time doing so) this PHP code. I wasn't going to bother, but this post is pretty high up on the front page. There's some XSS issues with the JSON output, the Content-Type header isn't set to 'application/json' so PHP decides to set it to 'text/html'. Now anyone that controls ipecho.net[0] or can execute commands as any user on the server[1] can XSS users of the panel. If you'd like to confirm, go to /sh/ps.php and notice where the page breaks due to strings in the JSON being interpreted as HTML. [0] https://github.com/afaqurk/linux-dash/blob/master/sh/ip.php#L6 https://github.com/afaqurk/linux-dash/blob/master/sh/ip.php#... [1] https://github.com/afaqurk/linux-dash/blob/master/sh/ps.php#L4 https://github.com/afaqurk/linux-dash/blob/master/sh/ps.php#...
  - deanclatworthy 13y ago
    But the XSS issues have nothing to do with the language choice. Python, ruby and any other langauge do nothing by default to protect you against such things either. I agree this is a poor choice of code, and an attack vector, but the language used here is not to blame.
    - anglebracket 13y ago
      I was responding to the parent's unwillingness to audit the code, not so much about technical issues with PHP. The biggest security issue that I've noticed with PHP is more cultural: Developers are far more likely to write ad-hoc pages with subtle security issues than use well-tested frameworks and libraries because it seems easier. I would never expect to see Ruby or Python code that generates a JSON array like this [0], but I'm not at all surprised when I see it in PHP. It's too easy and tempting to do the wrong thing. [0]: https://github.com/afaqurk/linux-dash/blob/master/sh/users.php#L5 https://github.com/afaqurk/linux-dash/blob/master/sh/users.p...
      - jdiez17 13y ago
- deanclatworthy 13y ago
  What you are talking about are core language decisions. Good developers have known how to work around register globals for years now, and it's not been a problem in any modern web app I've touched for the last 8 years (off the top of my head). The language being used here has absolutely nothing to do with the output. You could substitute the PHP shell_exec calls for something similar in python, ruby or any other language. You should audit anything that hits your server. Using PHP doesn't meant it's more susceptible to vulnerabilities.
  - jdiez17 13y ago
    >Good developers have known how to work around register globals for years now, and it's not been a problem in any modern web app I've touched for the last 8 years (off the top of my head). A good programming language lets a good developer to focus on writing code, not wrestling with bullshit decisions. YES, good programmers can write good and safe PHP code. But in the same way, bad or unexperienced programmers WILL and DO write awful code. The most recent example is OP's code. I've clicked on a few links that people replying to me have posted, and honestly, I'm glad I didn't spend any more time looking at it. >The language being used here has absolutely nothing to do with the output. You could substitute the PHP shell_exec calls for something similar in python, ruby or any other language. Like I said in previous comments, substituting PHP by Python or Ruby wouldn't improve the situation very much; I agree with that. The correct way to do it is to separate the data collection from the web interface. I think that should be common sense, and I fear that PHP is eroding that by making it very easy to do it "the lazy way" (fuck it, we'll use shell_exec on the web, what's the worst that could happen right?).