Como faço para que os programadores parem de escrever código vulnerável à injeção de SQL?

11

Às vezes, você fica ocupado e delega pequenas tarefas aos programadores juniores. Mas se você não prestar muita atenção, você se encontrará com esse tipo de código em produção:

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
        );
    }

}

Portanto, como removi a lógica de autenticação / gerenciamento de sessões por questões de brevidade, quem pode me dizer qual é o possível problema que pode haver com esse exemplo?

Roger Halliburton
fonte
Por que isso não é armazenado em um cookie simples?
quer
1
@ Darknight, você está assumindo que existe uma sessão para armazenar o cookie. Se existe ou não, é irrelevante para o problema de segurança maior.
21411 Roger Halliburton

Respostas:

24

Você pode ensiná-los. Todo mundo faz isso no começo, até você. Se esse tipo de código entra em produção, é culpa do pessoal sênior; não o júnior.

Editar:

Uma das coisas que eu fiz foi pessoalmente pedir às pessoas que ativamente revisassem meu código (incluindo os juniores) antes do lançamento. O código é revisado, os jovens vêem isso como uma experiência de aprendizado, as pessoas perdem o medo da revisão do código como um castigo e começam a fazer a mesma coisa.

John Kraft
fonte
2
+1 Tem que concordar. Você não pode culpar um (presumivelmente) programador júnior, porque assumiu um nível de conhecimento que ele pode não possuir. (Dito isto, você gosta de pensar essas questões são bem conhecidos neste dia e idade Então, novamente, eu. Gosta de pensar que eu era talentoso e com bom aspecto, etc.) :-)
John Parker
Uma declaração justa o suficiente. Acho que devo fazer uma pergunta de acompanhamento, perguntando por que o gerenciamento insistiria em lançar código em produção que não foi aprovado pelos programadores seniores. Arrisco meu trabalho devido a um pequeno problema de segurança?
Roger Halliburton
1
@Roger Halliburton: Bem, se esse problema de segurança que de alguma forma se explorada, o que é o pior que poderia acontecer? E por que apontar problemas de segurança custaria seu trabalho? A gerência não quer lidar com isso?
FrustratedWithFormsDesigner
1
@ Roger - é apenas menor até que você seja hackeado. :) Acho que colocar o código em produção sem uma revisão (não importa o nível do desenvolvedor) é um fracasso. Infelizmente, é uma prática muito comum em muitas empresas; e, normalmente, são necessários mais ou menos US $ 4! t antes que o gerenciamento comece a valorizar coisas como revisões de código.
John Kraft
1
@ Roger: Todo o código deve ser revisto, não apenas o código escrito por programadores "juniores". Até programadores seniores cometem erros.
Dean Harding
20

Hackeie o código na frente dos olhos e mostre como corrigi-lo. Repetidas vezes até que eles entendam.

Joe Phillips
fonte
4
Envie um e-mail para eles todas as manhãs: "A propósito, deixei cair seu banco de dados novamente ontem à noite por outra vulnerabilidade de injeção SQL que você deixou no seu código. Eu sei o quanto você gosta de restaurar backups de bancos de dados!: D"
Ant
2
@ Ant: Seja legal. Faça isso logo após o backup agendado, não antes.
Donal Fellows
@Donal: faça-o logo após o backup, diga-lhes que o backup falhou e deixe-os suar pelo resto do dia antes de restaurar o backup da noite para o dia.
jwenting
8

Você pode mandá-los para uma aula assim que ingressarem na sua empresa, antes que eles tenham acesso ao controle de origem, que os introduza a injeções de SQL, scripts entre sites, falsificação de solicitação entre sites e outras vulnerabilidades comuns. Cobrir exemplos cara a cara, quebrar códigos incorretos na frente deles, fazer com que eles quebrem códigos incorretos e aponte-os para o site da OWASP para obter mais informações quando eles se formarem.

Além disso, você pode determinar o uso de uma biblioteca personalizada que lida com isso para você, mas essa é apenas uma solução secundária, pois eles certamente executarão consultas personalizadas quando isso se tornar mais conveniente.

Se você tiver os recursos, garantir que membros mais altos da equipe verifiquem suas diferenças antes de confirmar também pode ser útil.

Conhecimento é poder!

yan
fonte
3
Eliminá-los antes que eles entrem na sua empresa é melhor. Se você estiver contratando alguém para escrever qualquer coisa que use SQL, não faça perguntas da entrevista sobre limites de injeção na incompetência.
Wooble
Eu geralmente concordo, mas um contrato júnior muito inteligente e ambicioso é muito, muito mais barato que um "desenvolvedor PHP com N anos de experiência", que não garante que seja muito melhor. Desenvolvedores juniores inteligentes podem pegar essas coisas rapidamente e ser um trunfo.
yan
3

Supondo que seja a insegurança a que outras pessoas se referiram, como desenvolvedor de qualquer nível, é fácil esquecer que o getPost () não está protegendo os dados primeiro.

Uma maneira de contornar isso é:

  1. Escreva uma classe que obtenha todos os dados POST / GET e grave-os como está em uma classe Singleton chamada 'insecure_data'. Em seguida, limpe as matrizes POST / GET.
  2. Desenvolvedores que precisam recuperar dados POST / GET da matriz 'insecure_data', e não matrizes POST / GET.

Qualquer desenvolvedor que recupere algo de um array chamado 'insecure_data' e não se preocupe em protegê-lo é ignorante ou preguiçoso. Se for o primeiro, forneça treinamento, após o qual deve ser o último - e então você terá uma questão disciplinar, não de programação.

Dan Blows
fonte
Uau, isso é desnecessariamente complicado e ainda não resolve o problema. Não é uma questão de limpar a entrada apenas por diversão, mas de limpar os dados que você está colocando no banco de dados, e esses dados poderiam, teoricamente, vir de qualquer lugar, de um formulário da Web, email ou documento XML que alguém envia. Em todos esses casos, você precisa limpar quando ele vai para o banco de dados.
Roger Halliburton
@RogerHalliburton É claro que isso não resolve tudo, mas é um conceito (um tipo de padrão de design, se você preferir), em vez de uma medida de segurança completa para fazer com que os dados inseguros pareçam inseguros.
Dan Blows
Ah entendo. Mas você está apenas lutando contra a natureza humana, se você disser a alguém "Este objeto é inseguro, a menos que seja sinalizado como seguro" e eles tentam usá-lo para alguma coisa, na maioria das vezes eles o sinalizam como seguro sem realmente verificar. Bem, aproveite o aumento da reputação.
Roger Halliburton
Do ponto de vista de um desenvolvedor júnior, apenas ver $ insecure_data gera uma flag de uma maneira que apenas ver $ postdata (ou o que for) não. A idéia vem do "Faça o código errado parecer errado", de Joel Spolsky . Se a natureza humana de seus desenvolvedores ignora essa bandeira vermelha, você precisa fornecer muito treinamento ou adquirir novos desenvolvedores.
Dan Blows
Não mantenho Spolsky em um pedestal. Esses são os desenvolvedores que ignoram facilmente as abas inconsistentes e não pensam duas vezes em usar algo chamado "inseguro".
111311 Roger Halliburton
1

Um dos melhores guias que li sobre segurança na web é este guia de segurança do Ruby on Rails . Embora seja Ruby on Rails, muitos dos conceitos se aplicam a qualquer desenvolvimento web. Eu encorajaria qualquer pessoa nova a ler esse guia.

mistagrooves
fonte
2
Todo mundo sabe que Ruby não é seguro.
21811 Roger Halliburton
5
@ Roger: "Todo mundo sabe" todos os tipos de coisas que podem ou não ser verdadeiras. Defendo uma abordagem de programação baseada na realidade, e não baseada em boatos.
Donal Fellows
0

O código que você vinculou acima é suscetível a um ataque de injeção SQL, porque as entradas HTTP que você está usando na consulta não foram limpas mysql_real_escape_stringou por qualquer outro meio.

Ryan
fonte
1
oh, eu pensei que nós estávamos responder a esta como uma questão de teste:]
Ryan
Os votos negativos são um pouco ruins, já que a redação da pergunta foi alterada ex post: P
Ryan
0

Em termos da sua pergunta (presumivelmente anuladora) "como faço para que os programadores parem de fazer isso", eu diria que os orientava regularmente, explicando cuidadosamente o problema em questão (e as possíveis consequências etc.) e enfatizando a importância das vulnerabilidades de código (tanto em termos de injeção de SQL quanto de scripts entre sites etc.) é provavelmente a solução mais sensata.

Se eles continuarem bagunçados, apesar de tudo o que foi mencionado acima (talvez você queira observar seus commits etc.) em vez de descobrir "ao vivo"), o problema é que você está falhando com eles como mentor ou que eles talvez precisem encontrar algo mais adequado para viver.

John Parker
fonte