0

This might seem silly but I am finding %20 character a column in my table. Its only in this column. While I post I am concatenating two text like $mtxt." sample text". The text in the column is like This%20sample%20text. The column is varchar and utf8 general collation. Other columns don't have this issues.

I am using jquery get method. like this.

$.get("https://xys.co.in/xys/xys.php/xys/"+xys+"/"+xya, function(data){});

Next in php

$urlParams = explode('/', $_SERVER['REQUEST_URI']); 
$functionName = $urlParams[x]; 
$functionName($urlParams,$conn);

function _xyz($urlParams,$conn){  
$xyz= mysqli_real_escape_string($urlParams[x]);
$sql ="Insert into `xyz` (x) values ('$xyz')"
if (mysqli_query($mysqli, $sql)) {
     echo "New record created successfully ".'$to'.'$s';
  } else {
    echo "Error: Insert " . mysqli_error($mysqli);
    exit();
  }
}
adbury
  • 37
  • 8
  • Show us how you save the data. Where do you get the data from? Are you sure it is not part of the input? – Dharman Jun 18 '20 at 10:04
  • Please update the question with jQuery and PHP code. Do not put extra details in comments. It is unreadable – Dharman Jun 18 '20 at 10:20
  • Did you know that you have just opened your whole server to remote code execution attacks? Still you have not shown us how you insert the data into the database. Where do you decode the URL parameters? – Dharman Jun 18 '20 at 10:42
  • 1
    Use `rawurldecode()` to decode the URL parts. However, the code you have shown us is really really bad and I recommend to delete it immediately and start over with a new idea. DO NOT USE THIS CODE! – Dharman Jun 18 '20 at 10:58
  • It is a very bad idea to use `die(mysqli_error($conn));` in your code, because it could potentially leak sensitive information. See this post for more explanation: [mysqli or die, does it have to die?](https://stackoverflow.com/a/15320411/1839439) – Dharman Jun 18 '20 at 10:59

1 Answers1

3

jQuery will be URL-encoding your text in order for it to make a valid URL (since URLs cannot contain spaces). A far more sensible way to send this kind of free-text data would be as body parameters in a POST request. Then you won't have issues like the one you've found, or problems with the length or the URL, or need to do any of the work splitting the URL string to get at the data. And you should also change your mysqli code so it's not vulnerable to SQL injection attacks etc.

Also you really should not allow the code to execute any arbitrary function name based on something in the URL. You need to whitelist the allowed action, at minimum, otherwise you can open yourself up to a whole world of vulnerabilities. And the action names should not directly reflect a function name in the PHP either.

Simple example:

jQuery:

$.post( //post instead of get
  "https://xys.co.in/xys/xys.php", 
  { "action": "xys", "xys": xys, "xya": xya }, //data object containing your parameters, to be passed in the request body
  function(response){ console.log(response); }
);

PHP:

if ($_SERVER['REQUEST_METHOD'] === 'POST') {
  $xys = $_POST["xys"];
  $xya = $_POST["xya"];
  $action = $_POST["action"];

  //whitelist valid actions
  switch($action) {
    case "xys":
        _xys($xys, $xya, $conn);
      break;
    default:
      echo "Invalid action";
      die();
  }
}

function _xys($xys, $xya, $conn) {
  $xyz = $xys.$xya;
  //use prepared statements and parameters to create clean, reliable and secure queries
  $sql = "insert into `xyz` (`x`) values (?)";
  $stmt = $conn->prepare($sql);
  $stmt->bind_param("s", $xyz);
  $stmt->execute();

  echo "New record created successfully";
}

Note also the lack of explicit error-handling here. Instead it's generally better to set mysqli to throw exceptions when errors occur and then let your program's exception handler deal with them (or use try/catch blocks if you need to). This leads to a lot less clutter in your code. It's also a bad idea to output exception details directly into the page in a live application - instead, log the exception details to the log file on the server, and output a generic user-friendly message on the front-end, without any technical details.

See here for more on prepared statements and paramterisation.

See here for more on the jQuery $.post function.

P.S. I've stuck with your variable names here for lack of anything else to use, but as a general point you should try to use meaningful variable names instead of abbreviations or random characters - it makes your code a lot more readable and maintainable.

ADyson
  • 57,178
  • 14
  • 51
  • 63
  • 1
    You get an upvote from me for a really thorough answer to an otherwise bad question. – Dharman Jun 18 '20 at 11:24
  • @ADyson Thank you for the answer. Actually I was not using the post function cos since there was quite a number of different types of requests happening using post would have me writing a lot of php files while the get method which I was using could include a lot of functions in one file. I will use the prepared statement method instead. – adbury Jun 18 '20 at 13:21
  • @Dharman The `rawurldecode()` worked. But had to remove the real escape method. Thank you. – adbury Jun 18 '20 at 13:23
  • 1
    @adbury _"there was quite a number of different types of requests happening using post would have me writing a lot of php files while the get method which I was using could include a lot of functions in one file"_ ...using POST does not give you this kind of restriction in any way at all. Look at the way I've written it - you can simply pass the necessary "action" in the request, and it can decide which function to run. It can all go through a single PHP script. There is no difference to your GET version in that respect. – ADyson Jun 18 '20 at 14:10