kaplanlior / midburn-queue

midburn.org tickets queue system
https://midburn.org
MIT License
2 stars 5 forks source link

order_json may be injected with malicious data! #18

Open danielkop opened 8 years ago

danielkop commented 8 years ago

The following curl may be sent by a client:

curl -X POST http://localhost:5000/register -d '{"email":"elad@gmail.com\", \"timestamp\" : \"injected timestamp\", \"injected field\" : \"injected data"}' --header "Content-Type: application/json"

Which will be resulted with:

[1] pry(#<MidburnQueue>)> params
=> {"email"=>"elad@gmail.com\", \"timestamp\" : \"malicous_timestamp\", \"blablabla\" : \""}
[2] pry(#<MidburnQueue>)> params["email"]
=> "elad@gmail.com\", \"timestamp\" : \"malicous_timestamp\", \"blablabla\" : \""
[3] pry(#<MidburnQueue>)>
[4] pry(#<MidburnQueue>)> order_json = %{{"ip":"#{request.ip}","timestamp":"#{Time.now.to_i}","email":"#{params[USERS_EMAIL_PARAM]}"}}
=> "{\"ip\":\"127.0.0.1\",\"timestamp\":\"1456955555\",\"email\":\"elad@gmail.com\", \"timestamp\" : \"malicous_timestamp\", \"blablabla\" : \"\"}"
[5] pry(#<MidburnQueue>)> order_json
=> "{\"ip\":\"127.0.0.1\",\"timestamp\":\"1456955555\",\"email\":\"elad@gmail.com\", \"timestamp\" : \"malicous_timestamp\", \"blablabla\" : \"\"}"
[6] pry(#<MidburnQueue>)> puts order_json
{"ip":"127.0.0.1","timestamp":"1456955555","email":"elad@gmail.com", "timestamp" : "malicous_timestamp", "blablabla" : ""}

and the following record in the queue:

{
  "ip": "127.0.0.1",
  "timestamp": "injected timestamp",
  "email": "elad@gmail.com",
  "injected field": "injected data"
}
eladg commented 8 years ago

Thanks @danielkop, that's a great find. We want to protect the queue and any data field in the system.

Important to note that regardless of this possible injection, the queue WILL NOT be changed or damaged.

yules commented 8 years ago

My suggestion - Use strong params in json. that way unknown params would be ignored

danielkop commented 8 years ago

@yules It's less an issue with the unknown parameter "injectedfield", and more an issue with the fact that you can override existing fields such as the ip and timestamp.

JSON.parse({"ip" : "goodip", "ip" : "badip"}) will result in {"ip" : "badip"}