0

Estoy desarrollando un sistema web en PHP,PDO,Ajax,Javascript, estoy manejando clases, pero no se si es la forma adecuada hoy en dia, estoy haciendolo de esta manera:

Primer achivo php, donde recibe el post :

include_once 'class/Estudiante.php';

    // Obtenemos el objecto

    $studentObj = new Estudiante();

    // Metodo post

    $_post = $_POST;

    $json = array();    

    $studentInfo = array();



if(!empty($_post['action']) && $_post['action']=="inscribir") {

        $studentObj->setStudentID($_post['estudiante_id']);

        
        $studentObj->setEdad($_post['Edad']);
        
        $studentObj->setNacimiento($_post['Fecha_Nacimiento']);
        
        $studentObj->setPassword($_post['Codigo']);

        

        

        $status = $studentObj->inscribir();
        if(!empty($status)){
            
            
            

            $json['msg'] = 'success';

            } else {

            $json['msg'] = 'failed';

        }

        header('Content-Type: application/json');   

        echo json_encode($json); 

    }

Segundo archivo donde se ecuentra la clase, los metodos set e inscribir()

try {
                 $cargo = "Ninguno";
             $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
             $RAMA= $this->_Rama;
             $integer = $this->_edad;
             $edad = (int)$integer;
            $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

             $data = [

                'edad' => $edad,
                
                'pass' => $this->_Password,
                
                'nacimiento' => $this->_Fecha_Nac,
                
                'inscripcion' => $fechaActual,
                
                'salida' => $salida
                
                
            ];

            $stmt = $this->db->prepare($sql);

            $stmt->execute($data);

            $status = $stmt->rowCount();
            

        } catch (Exception $err) {

            die("Error!: " . $err);

        }

Utilizo ajax para procesar la informacion que retorna el php:

 $("#student-frm").submit(function(e) {

  e.preventDefault();

  var formData = new FormData(this);

  var estudiante_id = jQuery('input#estudiante_id').val();

  var action = jQuery('input#action').val();

 

    $.ajax({

        type: "POST",

        data: formData,

        cache:false,

        contentType: false,

        processData: false,

        url: "accion.php",

        dataType: "json",

        beforeSend: function () {

        jQuery('button#student-btn').button('loading');

        },

        success: function(data) {

        jQuery('button#student-btn').button('reset');

        jQuery('#student-frm')[0].reset();

        jQuery('#create-student').modal('hide');

          if(action == 'inscribir'){

              swal("Registrado!", "El registro se guardo con éxito!", "success");

          } else {

              swal("Actualizado!", "Los cambios han sido guardado!", "success");

          }

        

        studentRecords.ajax.reload();

        },

        error: function() {

            swal("Error!", "Hubo un error, intente nuevamente!", "error");

        }

    });

});

Lo unico que quiero es evitar la inyeccion SQL, pero no se si lo estoy implementando bien, a lo que he estado leyendo este metodo es "Marcadores de posicion por nombres", espero que me puedan brindar una retroalimentación

6
  • 1
    El código que muestras aqui no tiene ningún riesgo de Inyección SQL.
    – A. Cedano
    Commented el 1 feb. 2022 a las 20:20
  • El metodo si es marcadores de posicion cierto? Me confunden ciertos conceptos Commented el 1 feb. 2022 a las 20:29
  • 1
    Lo que tú estás usando se llama marcadores de parámetros con nombre, y cuando se usa ? se llama marcadores de posición (porque se marca una posición para cada valor con ?). Revisa la documentación está claramente explicado allí.
    – A. Cedano
    Commented el 1 feb. 2022 a las 21:41
  • Muchas Gracias amigo :), me has despejado las dudas Commented el 1 feb. 2022 a las 21:53
  • Los marcadores ? son más prácticos en algunos contextos, pero son más peligrosos, no en el sentido de la inyección sino en que, si no tienes cuidado al pasar los parámetros, puedes poner valores en una posición equivocada, haciendo que los datos sean erróneos. Con los marcadores con nombre siempre sabrás a quién estás pasando cada valor, porque debes poner el nombre al lado del valor.
    – A. Cedano
    Commented el 1 feb. 2022 a las 21:56

1 respuesta 1

2

Tu código no presenta ningún riesgo de inyección SQL, pues estás usando correctamente las consultas preparadas y pasando correctamente los valores, los cuales se pueden pasar usando los métodos bindParam(), bindValue() o en el método execute(). La inyección SQL sólo es posible si tú pasas los valores directamente en la instrucción SQL que uses, sea mediante prepare() sea mediante query(). Al poner marcadores (de :nombre o de posición ?, sin mezclar), en la instrucción SQL evitas que un usuario mal intencionado inyecte código a través de lo que serían los valores de la consulta.

Sin embargo, en otros aspectos tu código es farragoso, repetitivo y no controla datos esenciales. Conviene por ejemplo que si alguna de las columnas tiene una restricción NOT NULL, verifiques que los valores para esas columnas contengan datos y evites la inserción, respetando así el contrato de NOT NULL en la base de datos y escribiendo un código coherente.

Conviene igualmente que verifiques cada dato que vendría en el $_POST, el usuario pudo haber mandando algunos datos vacíos, y te estarías exponiendo al menos a uno o más casos de Undefined index. La verificación del action también se puede simplificar. Y la coherencia del código en sí misma no se aprecia, por ejemplo, no veo si retornas algo desde el contexto en que insertas.

Dado que dices que haces un return $status te propondré una forma controlada de retornar lo que haya ocurrido en el método, usaré un array, porque te permite trabajar con pares clave => valor que puedes luego verificar y mostrar mensajes. Esto es más fácil de adaptar a una política de manejo de errores, o a respuestas de una API, etc.

Aplicando un poco lo dicho podemos mejorar el código de este modo:

Donde recibes el $_POST

include_once 'class/Estudiante.php';

#Declarar cada variable sólo cuando estés seguro de que la usarás
$json = array();

#¿Para qué es esta variable?    
$studentInfo = array();
/*
   Si usas PHP anterior a la v. 7
   cambia esto por:
   $action = !empty($_POST['action']) ? $_POST['action'] : null;
*/
$action=$_POST['action'] ?? null;
if ($action && $action == 'inscribir') {
    /*
       Aquí vamos a verificar cada dato para evitar
       casod de Undefined index
       Si tienes PHP v. anterior a 7 usa lo que
       ya se indicó más arriba para el action en todas
       las asignaciones que siguen
       Aquí convendría un if para verificar todas aquellas
       columnas que tiene restriccion NOT NULL
       Usaré una convención de nombre en inglés,
       entre otras ventajas, te evitarías 
       nombre de variables kilométricos, por ejemplo
       dob = date of birth (fecha de nacimiento)
       */
    
    $id=$_POST['estudiante_id'] ?? null;
    $age=$_POST['Edad'] ?? null;
    $dob=$_POST['Fecha_Nacimiento'] ?? null;
    $code=$_POST['Codigo'] ?? null;

    /*
       Aquí verificarías con un if TODOS
       los NOT NULL, por ejemplo
       if ($id && $code) {  // insertar } else { //faltan datos obligatorios }
       */

    $studentObj = new Estudiante();
    $studentObj->setStudentID($id);   
    # ¿Por qué la edad no es un campo calculado?    
    $studentObj->setEdad($age);
    $studentObj->setNacimiento($dob);        
    $studentObj->setPassword($code);

    /*
       OJO A ESTO:
       En la nueva versión del código
       inscribir devolverá un array con dos posibilidades:
       1. Siempre una clave status que será true o false
       2. Una clave msg con un mensaje
       De este modo, la respuesta del método
       queda acoplada a la lógica que ya estabas siguiendo
       aunque quizá convendría refinarla un poco
       adpotando lo de la clave true o false           
       */ 
    $json = $studentObj->inscribir();


} else {
    $json['msg'] = 'No se ha enviado la acción inscribir';
}

header('Content-Type: application/json');   
echo json_encode($json); 

Donde haces la inserción

try {
        $cargo = "Ninguno";
        $fechaActual = date('Y-m-d');
        $salida = date('Y-m-d',strtotime($fecha_actual.'+ 1 year'));
        $RAMA= $this->_Rama;
        $integer = $this->_edad;
        $edad = (int)$integer;
        $sql = "INSERT INTO student(edad, password, fecha_nacimiento, fecha_creacion,fecha_salida)  VALUES (:edad, :pass ,:nacimiento, :inscripcion,:salida)";

         /*
             Vamos a poner los dos puntos a los marcadores
             de todos modos PDO lo haría por nosotros
             pero si se lo damos hecho mejor,
             así lo hacemos trabajar menos
         */
         $data = [
                ':edad' => $edad,               
                ':pass' => $this->_Password,               
                ':nacimiento' => $this->_Fecha_Nac,               
                ':inscripcion' => $fechaActual,                
                ':salida' => $salida                               
            ];

          $stmt = $this->db->prepare($sql);
          $stmt->execute($data);
          /*
             Como dije más arriba, aquí vamos a devolver
             un array con una clave status que será true
             y una clave msg que será un mensaje
             
             con la cantidad de filas afectadas
             */
             $status = array(
                               'status'=>true,
                               'msg' => sprintf("Se insertaron %s filas",$stmt->rowCount())
                             );            
        } catch (Exception $err) {          
            /*
               Aquí, para seguir con el contrato de nuestro código
               vamos a devolver un array con una clave status que es false
               y siempre una clave msg que será el mensaje de error
               En un contexto de Política de manejo de errores más avanzada
               podrías recoger el código de error por ejemplo
               e incorporarlo a un sistema de errores personalizados o cosas así ...
               Lo que es importante aquí es evitar
               que se muestren mensajes de error interno
               los cuales son MUY jugosos para los hackers
               Por ejemplo, un gravísimo fallo de PDO es que
               ante errores de conexión ¡revela la contraseña
               en el mensaje de error!
               */
            
            $status = array(
                               'status'=>false,
                               'msg' => 'Error ejecutando la consulta. Si quieres captura el código de error e incorpóralo en tu PME'
                             );        
            return $status; 

        }
5
  • En el try catch hago un $status = $stmt->rowCount(); return $status; , que puedo utilizar en el Exception para evitar mostrar los errores internos, quitar el die y como que puedo poner? Commented el 4 feb. 2022 a las 22:08
  • 1
    @karlDomínguez ahora te lo explico en la respuesta. No vi el return de $status por lo que no sabía de qué modo estabas retornando la información, de ahí mi extrañeza al respecto.
    – A. Cedano
    Commented el 4 feb. 2022 a las 22:17
  • Tienes alguna sugerenia de evitar mostrar los errores internos? Commented el 4 feb. 2022 a las 22:40
  • @karlDomínguez revisa el código editado. Se ha modificado en dos partes: 1. Donde recibes la respuesta de la llamada a inscribir y 2. en los dos bloques de inscribir. Lo básico es que se devolverá un array con status igual a true cuando todo vaya OK y un status igual a false cuando haya algún problema. A partir de ahí tú podrías capturar el código de error por ejemplo e incorporarlo a tu PME, pero ese es ya otro asunto...
    – A. Cedano
    Commented el 4 feb. 2022 a las 22:44
  • Esta muy bien tu observación de como retornar la informacion, pero la proceso en ajax, de la siguiente manera (ya lo puse arriba), crees que pueda aplicar la forma en que lo planteas? Commented el 5 feb. 2022 a las 2:57

Tu Respuesta

By clicking “Publica tu respuesta”, you agree to our terms of service and acknowledge you have read our privacy policy.

¿No es la respuesta que buscas? Examina otras preguntas con la etiqueta o formula tu propia pregunta.