¿Cómo puedo hacer que los programadores dejen de escribir código vulnerable a la inyección de SQL?

11

A veces se ocupa y delega tareas pequeñas a programadores junior. Pero si no presta suficiente atención, se encuentra con este tipo de código en producción:

class DivtoggleController extends Zend_Controller_Action {

    public function closeAction() {
        /* ... code removed for brevity ... */

        $req = $this->getRequest();
        $formData = $req->getPost();

        $d = $formData['div'];
        $i = $formData['id'];

        $dm = new Model_DivtoggleManager();
        $rs = $dm->setDivToggleById($d, $i);

    }

}


class Model_DivtoggleManager extends Zend_Db_Table {

    public function setDivToggleById($div, $id) {
        $result = $this->getAdapter()->query(
           "update div_toggle set " . $div . "=1 where id=" . $id
        );
    }

}

Entonces, dado que he eliminado la lógica de administración de autenticación / sesión por brevedad, ¿quién puede decirme qué posible problema podría haber con esta muestra?

Roger Halliburton
fuente
¿Por qué esto no se almacena en una cookie simple?
Darknight
1
@Darknight, está asumiendo que hay una sesión en la que almacenar la cookie. Si existe o no es irrelevante para el problema de seguridad más grande.
Roger Halliburton

Respuestas:

24

Puedes enseñarles. Todo el mundo hace esto al principio, incluso tú. Si este tipo de código llega a producción, es culpa de la gente mayor; No el junior.

Editar:

Una de las cosas que he hecho es que personalmente he tomado la iniciativa de pedirles a las personas que revisen mi código (incluidos los junior) antes de un lanzamiento. El código se revisa, los jóvenes lo ven como una experiencia de aprendizaje, las personas pierden el miedo a la revisión del código como un castigo y comienzan a hacer lo mismo.

John Kraft
fuente
2
+1 Tengo que estar de acuerdo. No puede culpar a un programador junior (presumiblemente) porque ha asumido un nivel de conocimiento que puede no poseer. (Dicho esto, te gusta pensar estos temas son bien conocidos en este día y edad Por otra parte, me. Me gusta pensar que yo tenía talento y de buen aspecto, etc.) :-)
John Parker
Una declaración bastante justa. Supongo que debería hacer una pregunta de seguimiento preguntando por qué la gerencia insistiría en lanzar código en producción que no haya sido aprobado por los programadores senior. ¿Arriesgo mi trabajo por un problema de seguridad menor?
Roger Halliburton
1
@Roger Halliburton: Bueno, si ese problema de seguridad no alguna manera son explotados, lo que es lo peor que podría suceder? ¿Y por qué señalar problemas de seguridad le costaría su trabajo? ¿La gerencia no quiere que se solucione esto?
FrustratedWithFormsDesigner
1
@Roger: solo es menor hasta que te hackean. :) Creo que poner el código en producción sin una revisión (sin importar el nivel del desarrollador) es un fracaso. Desafortunadamente, es una práctica muy común en muchas empresas; e, es decir, generalmente se necesitan unos importantes $ 4! t antes de que la gerencia comience a valorar cosas como las revisiones de código.
John Kraft
1
@Roger: Todo el código debe ser revisado, no solo el código escrito por programadores "junior". Incluso los programadores senior cometen errores.
Dean Harding
20

Hackea su código frente a sus ojos y luego muéstrales cómo solucionarlo. Una y otra vez hasta que entiendan.

Joe Phillips
fuente
44
Envíelos por correo electrónico todas las mañanas: "Por cierto, dejé caer su base de datos nuevamente anoche a través de otra vulnerabilidad de inyección SQL que dejó en su código. ¡Sé cuánto le gusta restaurar las copias de seguridad de la base de datos!: D"
Ant
2
@Ant: Sé amable. Hágalo poco después de la copia de seguridad programada, no poco antes.
Donal Fellows
@Donal: hágalo poco después de la copia de seguridad, luego dígales que la copia de seguridad falló y déjelos sudar por el resto del día antes de restaurar la copia de seguridad durante la noche.
Jwenting
8

Puede ordenarles que tomen una clase tan pronto como se unan a su empresa, antes de que tengan acceso de control de fuente, que les presenta inyecciones SQL, secuencias de comandos entre sitios, falsificación de solicitudes entre sitios y otras vulnerabilidades comunes. Cubra los ejemplos cara a cara, descifre el código incorrecto frente a ellos, haga que descifren el código incorrecto y diríjalos al sitio de OWASP para obtener más información una vez que se "gradúen".

Además, puede ordenar el uso de una biblioteca personalizada que maneje esto por usted, pero esa es solo una solución secundaria, ya que se asegurarán de ejecutar consultas personalizadas cuando sea más conveniente.

Si tiene los recursos, también puede ser útil garantizar que más miembros senior del equipo verifiquen sus diferencias antes de comprometerse.

¡El conocimiento es poder!

Yan
fuente
3
Es mejor eliminarlos antes de que se unan a su empresa. Si está contratando a alguien para que escriba algo que use SQL, no haga preguntas de entrevista sobre límites de inyección en la incompetencia.
Wooble
En general, estoy de acuerdo, pero una contratación junior muy inteligente y ambiciosa es mucho, mucho más barata que un "desarrollador de PHP con N años de experiencia", eso no garantiza que sea mucho mejor. Los desarrolladores junior inteligentes pueden aprender estas cosas muy rápido y ser un gran activo.
Yan
3

Asumiendo que es la inseguridad a la que otros se han referido, como desarrollador de cualquier nivel, es fácil olvidar que getPost () no está asegurando los datos primero.

Una forma de evitar esto es:

  1. Escriba una clase que obtenga todos los datos POST / GET y los escriba como está en una clase Singleton llamada 'insecure_data'. Luego borre las matrices POST / GET.
  2. Desarrolladores que tienen que recuperar datos POST / GET de la matriz 'insecure_data', no las matrices POST / GET.

Cualquier desarrollador que recupere algo de una matriz llamada 'insecure_data' y no se moleste en asegurarlo es ignorante o vago. Si es lo primero, proporcione capacitación, después de lo cual debe ser lo último, y luego tiene un problema disciplinario, no de programación.

Dan golpes
fuente
Wow, eso es innecesariamente complicado y aún no resuelve el problema. No se trata de eliminar la entrada solo por diversión, sino de eliminar los datos que está poniendo en la base de datos, y esos datos podrían provenir teóricamente de cualquier lugar, de un formulario web, un correo electrónico o un documento XML que alguien sube En todos estos casos, necesita restregar cuando va a la base de datos.
Roger Halliburton
@RogerHalliburton Por supuesto, no resuelve todo, pero es un concepto (una especie de patrón de diseño, por así decirlo) en lugar de una medida de seguridad completa para hacer que los datos inseguros parezcan inseguros.
Dan Blows
Ah, ya veo. Pero solo estás luchando contra la naturaleza humana, si le dices a alguien "Este objeto es inseguro a menos que se marque como seguro" e intentan usarlo para algo, la mayoría de las veces lo marcarán como seguro sin verificarlo. Bueno, disfruta el aumento de reputación.
Roger Halliburton
Desde el punto de vista de un desarrollador junior, solo ver $ insecure_data levanta una bandera de una manera que solo ver $ postdata (o lo que sea) no lo hace. La idea proviene de "Hacer que el código incorrecto parezca incorrecto" de Joel Spolsky . Si la naturaleza humana de sus desarrolladores es tal que ignore esa señal de alerta, entonces debe proporcionar mucha capacitación u obtener nuevos desarrolladores.
Dan Blows
No mantengo a Spolsky en un pedestal. Estos son desarrolladores que ignoran fácilmente las pestañas inconsistentes, no van a pensar dos veces antes de usar algo que se llama "inseguro".
Roger Halliburton
1

Una de las mejores guías que he leído sobre seguridad web es esta guía de seguridad de Ruby on Rails . Aunque es Ruby on Rails, muchos de los conceptos se aplican a cualquier desarrollo web. Animaría a cualquiera nuevo a leer esa guía.

equivocados
fuente
2
Todos saben que Ruby no es seguro.
Roger Halliburton
55
@Roger: "Todo el mundo sabe" todo tipo de cosas que podrían ser ciertas o no. Abogo por un enfoque de la programación basado en la realidad, no uno basado en rumores.
Donal Fellows
0

El código que ha vinculado anteriormente es susceptible a un ataque de inyección SQL, porque las entradas HTTP que está utilizando en la consulta no se han limpiado mysql_real_escape_stringni por ningún otro medio.

Ryan
fuente
1
oh, pensé que estábamos respondiendo esto como una pregunta de prueba:]
Ryan
Los votos negativos son un poco flojos, ya que la redacción de la pregunta se cambió ex post: P
Ryan
0

En términos de su (supuestamente primordial) "¿cómo puedo hacer que los programadores dejen de hacer esto?", Diría que los asesora regularmente, explicando cuidadosamente el problema en cuestión (y las posibles consecuencias, etc.) y enfatizando La importancia de las vulnerabilidades de código (tanto en términos de inyección de SQL como scripting entre sitios, etc.) es probablemente la solución más sensata.

Si siguen estropeando a pesar de todo lo anterior (es posible que desee echar un vistazo a sus compromisos, etc. en lugar de descubrir "en vivo"), entonces el problema es que les está fallando como mentor, o que quizás necesiten encontrar algo más adecuado para ganarse la vida.

John Parker
fuente