From cade8e25e767a8fb2b4581badada0f24a505215a Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Tue, 27 Aug 2013 10:40:50 +0200 Subject: user_logged_in(): always load muser Signed-off-by: Florian Pritz --- application/helpers/filebin_helper.php | 1 + 1 file changed, 1 insertion(+) (limited to 'application') diff --git a/application/helpers/filebin_helper.php b/application/helpers/filebin_helper.php index 4af106a14..57f7bdc65 100644 --- a/application/helpers/filebin_helper.php +++ b/application/helpers/filebin_helper.php @@ -326,6 +326,7 @@ function auth_driver_function_implemented($function) function user_logged_in() { $CI =& get_instance(); + $CI->load->model("muser"); return $CI->muser->logged_in(); } -- cgit v1.2.3-24-g4f1b From f8417cd3aa92f49cbe98188cd6fca2ec50da9613 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Wed, 28 Aug 2013 00:08:29 +0200 Subject: muser: always verify api credentials; improve error messages Signed-off-by: Florian Pritz --- application/models/muser.php | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'application') diff --git a/application/models/muser.php b/application/models/muser.php index 843b7cad6..b3c16bf78 100644 --- a/application/models/muser.php +++ b/application/models/muser.php @@ -67,14 +67,6 @@ class Muser extends CI_Model { { $username = $this->input->post("username"); $password = $this->input->post("password"); - $apikey = $this->input->post("apikey"); - - if ($apikey !== false) { - if ($this->apilogin(trim($apikey))) { - return true; - } - show_error("API key login failed", 401); - } // prefer post parameters if either (username or password) is set if ($username === false && $password === false) { @@ -84,19 +76,24 @@ class Muser extends CI_Model { } } - if ($apikey === false && $username !== false && $password !== false) { + if ($username !== false && $password !== false) { if ($this->login($username, $password)) { return true; } else { show_error("Login failed", 401); } } + + return null; } function apilogin($apikey) { $this->require_session(); + // get rid of spaces and newlines + $apikey = trim($apikey); + $query = $this->db->query(" SELECT a.user userid FROM apikeys a @@ -111,7 +108,7 @@ class Muser extends CI_Model { return true; } - return false; + show_error("API key login failed", 401); } function logout() @@ -168,22 +165,26 @@ class Muser extends CI_Model { return true; } - show_error("Access denied", 403); + show_error("Access denied: Access level too low", 403); } function require_access($wanted_level = "full") { + if ($this->input->post("apikey") !== false) { + $this->apilogin($this->input->post("apikey")); + } + + if (is_cli_client()) { + $this->login_cli_client(); + } + if ($this->logged_in()) { return $this->check_access_level($wanted_level); } + // if a CLI client reaches this point it failed to log in if (is_cli_client()) { - if ($this->login_cli_client()) { - return $this->check_access_level($wanted_level); - } - - echo "FileBin requires you to have an account, please go to the homepage for more information.\n"; - exit(); + show_error("Not authenticated. FileBin requires you to have an account, please go to the homepage for more information.\n", 401); } // desktop clients get redirected to the login form -- cgit v1.2.3-24-g4f1b From 84ce2c6ce0eb1b4f2f32c4ae0d7e08f3571f5018 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Wed, 14 Aug 2013 17:06:07 +0200 Subject: Provide json output for api functions Signed-off-by: Florian Pritz --- application/controllers/file.php | 64 +++++++++++++---------- application/controllers/user.php | 8 +++ application/core/MY_Controller.php | 12 ++++- application/errors/error_general.php | 10 ++++ application/helpers/filebin_helper.php | 19 ++++++- application/views/file/deleted.php | 9 ++-- application/views/file/too_big.php | 3 -- application/views/file/upload_error.php | 6 --- application/views/file_plaintext/too_big.php | 2 - application/views/file_plaintext/upload_error.php | 2 - 10 files changed, 84 insertions(+), 51 deletions(-) delete mode 100644 application/views/file/too_big.php delete mode 100644 application/views/file/upload_error.php delete mode 100644 application/views/file_plaintext/too_big.php delete mode 100644 application/views/file_plaintext/upload_error.php (limited to 'application') diff --git a/application/controllers/file.php b/application/controllers/file.php index ef2e87084..c336db92b 100644 --- a/application/controllers/file.php +++ b/application/controllers/file.php @@ -9,6 +9,12 @@ class File extends MY_Controller { + protected $json_enabled_functions = array( + "upload_history", + "do_upload", + "do_delete", + ); + function __construct() { parent::__construct(); @@ -280,6 +286,10 @@ class File extends MY_Controller { } } + if (request_type() == "json") { + return send_json_reply($this->data["urls"]); + } + if (is_cli_client()) { $redirect = false; } @@ -456,7 +466,7 @@ class File extends MY_Controller { ORDER BY date $order ", array($user))->result_array(); - if ($this->input->get("json") !== false) { + if (request_type() == "json") { return send_json_reply($query); } @@ -499,11 +509,11 @@ class File extends MY_Controller { $ids = $this->input->post("ids"); $errors = array(); - $msgs = array(); + $deleted = array(); $deleted_count = 0; $total_count = 0; - if (!$ids) { + if (!$ids || !is_array($ids)) { show_error("No IDs specified"); } @@ -511,20 +521,34 @@ class File extends MY_Controller { $total_count++; if (!$this->mfile->id_exists($id)) { - $errors[] = "'$id' didn't exist anymore."; + $errors[] = array( + "id" => $id, + "reason" => "doesn't exist", + ); continue; } if ($this->mfile->delete_id($id)) { - $msgs[] = "'$id' has been removed."; + $deleted[] = $id; $deleted_count++; } else { - $errors[] = "'$id' couldn't be deleted."; + $errors[] = array( + "id" => $id, + "reason" => "unknown error", + ); } } + if (request_type() == "json") { + return send_json_reply(array( + "errors" => $errors, + "deleted" => $deleted, + "total_count" => $total_count, + "deleted_count" => $deleted_count, + )); + } + $this->data["errors"] = $errors; - $this->data["msgs"] = $msgs; $this->data["deleted_count"] = $deleted_count; $this->data["total_count"] = $total_count; @@ -571,20 +595,11 @@ class File extends MY_Controller { $filename = "stdin"; if (!$content) { - $this->output->set_status_header(400); - $this->data["msg"] = "Nothing was pasted, content is empty."; - $this->load->view('header', $this->data); - $this->load->view($this->var->view_dir.'/upload_error', $this->data); - $this->load->view('footer'); - return; + show_error("Nothing was pasted, content is empty.", 400); } if ($filesize > $this->config->item('upload_max_size')) { - $this->output->set_status_header(413); - $this->load->view('header', $this->data); - $this->load->view($this->var->view_dir.'/too_big'); - $this->load->view('footer'); - return; + show_error("Error while uploading: File too big", 413); } $limits = $this->muser->get_upload_id_limits(); @@ -624,8 +639,6 @@ class File extends MY_Controller { foreach ($files as $key => $file) { // getNormalizedFILES() removes any file with error == 4 if ($file['error'] !== UPLOAD_ERR_OK) { - $this->output->set_status_header(400); - // ERR_OK only for completeness, condition above ignores it $errors = array( UPLOAD_ERR_OK => "There is no error, the file uploaded with success", @@ -646,19 +659,12 @@ class File extends MY_Controller { $this->data["msg"] = "Unknown error code: ".$file['error'].". Please report a bug."; } - $this->load->view('header', $this->data); - $this->load->view($this->var->view_dir.'/upload_error', $this->data); - $this->load->view('footer'); - return; + show_error("Error while uploading: ".$this->data["msg"], 400); } $filesize = filesize($file['tmp_name']); if ($filesize > $this->config->item('upload_max_size')) { - $this->output->set_status_header(413); - $this->load->view('header', $this->data); - $this->load->view($this->var->view_dir.'/too_big'); - $this->load->view('footer'); - return; + show_error("Error while uploading: File too big", 413); } } diff --git a/application/controllers/user.php b/application/controllers/user.php index 498a626d7..56f571d6a 100644 --- a/application/controllers/user.php +++ b/application/controllers/user.php @@ -8,6 +8,10 @@ */ class User extends MY_Controller { + protected $json_enabled_functions = array( + "apikeys", + ); + function __construct() { @@ -127,6 +131,10 @@ class User extends MY_Controller { WHERE `user` = ? order by created desc ", array($userid))->result_array(); + if (request_type() == "json") { + return send_json_reply($query); + } + $this->data["query"] = $query; $this->load->view('header', $this->data); diff --git a/application/core/MY_Controller.php b/application/core/MY_Controller.php index 278768ad2..3ee63424a 100644 --- a/application/core/MY_Controller.php +++ b/application/core/MY_Controller.php @@ -11,7 +11,7 @@ class MY_Controller extends CI_Controller { public $data = array(); public $var; - private $json_enabled_functions = array( + protected $json_enabled_functions = array( ); function __construct() @@ -31,6 +31,16 @@ class MY_Controller extends CI_Controller { mb_internal_encoding('UTF-8'); $this->load->helper(array('form', 'filebin')); + if (isset($_SERVER["HTTP_ACCEPT"])) { + if ($_SERVER["HTTP_ACCEPT"] == "application/json") { + request_type("json"); + } + } + + if (request_type() == "json" && ! in_array($this->uri->rsegment(2), $this->json_enabled_functions)) { + show_error("Function not JSON enabled"); + } + $this->data['title'] = "FileBin"; } } diff --git a/application/errors/error_general.php b/application/errors/error_general.php index da3999cbd..fc3d3f607 100755 --- a/application/errors/error_general.php +++ b/application/errors/error_general.php @@ -9,6 +9,16 @@ if (class_exists("CI_Controller") && !isset($GLOBALS["is_error_page"])) { $CI->load->helper("filebin"); $CI->load->helper("url"); + if (request_type() == "json") { + $array = array( + "status" => "error", + "message" => strip_tags($message), + ); + header('Content-type: application/json'); + echo json_encode($array); + exit(); + } + if (is_cli_client()) { $message = strip_tags($message); echo "$heading: $message\n"; diff --git a/application/helpers/filebin_helper.php b/application/helpers/filebin_helper.php index 57f7bdc65..9b124506f 100644 --- a/application/helpers/filebin_helper.php +++ b/application/helpers/filebin_helper.php @@ -330,11 +330,26 @@ function user_logged_in() return $CI->muser->logged_in(); } -function send_json_reply($array) +function send_json_reply($array, $status = "success") { + $reply = array(); + $reply["status"] = $status; + $reply["data"] = $array; + $CI =& get_instance(); $CI->output->set_content_type('application/json'); - $CI->output->set_output(json_encode($array)); + $CI->output->set_output(json_encode($reply)); +} + +function request_type($set_type = null) +{ + static $type = null; + + if ($set_type !== null) { + $type = $set_type; + } + + return $type; } # vim: set noet: diff --git a/application/views/file/deleted.php b/application/views/file/deleted.php index ef02398d9..8a5818f2d 100644 --- a/application/views/file/deleted.php +++ b/application/views/file/deleted.php @@ -1,12 +1,9 @@
"; - echo implode("
\n", $errors); - echo "

"; - } ?> - "; - echo implode("
\n", $msgs); + foreach ($errors as $error) { + echo "${error["id"]}: ${error["reason"]}
\n"; + } echo "

"; } ?> diff --git a/application/views/file/too_big.php b/application/views/file/too_big.php deleted file mode 100644 index 5b970a4c8..000000000 --- a/application/views/file/too_big.php +++ /dev/null @@ -1,3 +0,0 @@ -
-

Sorry, the file you uploaded is too big.

-
diff --git a/application/views/file/upload_error.php b/application/views/file/upload_error.php deleted file mode 100644 index 83968eec2..000000000 --- a/application/views/file/upload_error.php +++ /dev/null @@ -1,6 +0,0 @@ -
-

- An error occurred while uploading.
- -

-
diff --git a/application/views/file_plaintext/too_big.php b/application/views/file_plaintext/too_big.php deleted file mode 100644 index d27a0295c..000000000 --- a/application/views/file_plaintext/too_big.php +++ /dev/null @@ -1,2 +0,0 @@ -Sorry, the file you uploaded is too big. - diff --git a/application/views/file_plaintext/upload_error.php b/application/views/file_plaintext/upload_error.php deleted file mode 100644 index c86c56911..000000000 --- a/application/views/file_plaintext/upload_error.php +++ /dev/null @@ -1,2 +0,0 @@ -An error occurred while uploading. - -- cgit v1.2.3-24-g4f1b From 285262b6c668b4f367f8222880ceb01be39fd3ac Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Thu, 29 Aug 2013 17:55:52 +0200 Subject: Add CSRF protection Signed-off-by: Florian Pritz --- application/config/config.php | 2 +- application/core/MY_Controller.php | 46 +++++++++++++++++++++++++++++ application/views/file/client.php | 4 +-- application/views/file_plaintext/client.php | 4 +-- 4 files changed, 51 insertions(+), 5 deletions(-) (limited to 'application') diff --git a/application/config/config.php b/application/config/config.php index dda82de97..4aadac68d 100644 --- a/application/config/config.php +++ b/application/config/config.php @@ -293,7 +293,7 @@ $config['global_xss_filtering'] = FALSE; | 'csrf_cookie_name' = The cookie name | 'csrf_expire' = The number in seconds the token should expire. */ -$config['csrf_protection'] = FALSE; +$config['csrf_protection'] = FALSE; // our controller enables this later $config['csrf_token_name'] = 'csrf_test_name'; $config['csrf_cookie_name'] = 'csrf_cookie_name'; $config['csrf_expire'] = 7200; diff --git a/application/core/MY_Controller.php b/application/core/MY_Controller.php index 3ee63424a..09b813b71 100644 --- a/application/core/MY_Controller.php +++ b/application/core/MY_Controller.php @@ -19,6 +19,7 @@ class MY_Controller extends CI_Controller { parent::__construct(); $this->var = new StdClass(); + $csrf_protection = true; $this->load->library('migration'); if ( ! $this->migration->current()) { @@ -41,6 +42,51 @@ class MY_Controller extends CI_Controller { show_error("Function not JSON enabled"); } + if ($this->input->post("apikey") !== false) { + /* This relies on the authentication code always verifying the supplied + * apikey. If the key is not verified/logged in an attacker could simply + * add an empty "apikey" field to the CSRF form to circumvent the + * protection. If we always log in if a key is supplied we can ensure + * that an attacker (and the victim since they get a cookie) can only + * access the attacker's account. + */ + $csrf_protection = false; + } + + $uri_start = $this->uri->rsegment(1)."/".$this->uri->rsegment(2); + $csrf_whitelisted_handlers = array( + "always" => array( + /* Whitelist the upload pages because they don't cause harm and a user + * might keep the upload page open for more than csrf_expire seconds + * and we don't want to annoy them when they upload a big file and the + * CSRF check fails. + */ + "file/do_upload", + "file/do_paste", + ), + "cli_client" => array( + "file/do_delete", + "file/delete", + "file/upload_history", + ), + ); + if (in_array($uri_start, $csrf_whitelisted_handlers["always"])) { + $csrf_protection = false; + } + + // TODO: replace cli client with request_type("plain")? + if (is_cli_client() && in_array($uri_start, $csrf_whitelisted_handlers["cli_client"])) { + $csrf_protection = false; + } + + if ($csrf_protection) { + // 2 functions for accessing config options, really? + $this->config->set_item('csrf_protection', true); + config_item("csrf_protection", true); + $this->security->__construct(); + $this->security->csrf_verify(); + } + $this->data['title'] = "FileBin"; } } diff --git a/application/views/file/client.php b/application/views/file/client.php index 5e141f141..29e254a80 100644 --- a/application/views/file/client.php +++ b/application/views/file/client.php @@ -42,7 +42,7 @@ machine login my_username password my_secret_password

Shell

-curl -n -F "file=@/home/user/foo"    (binary safe)
-cat file | curl -n -F "file=@-;filename=stdin"    (binary safe)
+curl -n -F "file=@/home/user/foo"    (binary safe)
+cat file | curl -n -F "file=@-;filename=stdin"    (binary safe)
diff --git a/application/views/file_plaintext/client.php b/application/views/file_plaintext/client.php index b37fd81bd..0ab556df2 100644 --- a/application/views/file_plaintext/client.php +++ b/application/views/file_plaintext/client.php @@ -1,6 +1,6 @@ Shell (binary safe): - curl -n -F "file=@/home/user/foo" - cat file | curl -n -F "file=@-;filename=stdin" + curl -n -F "file=@/home/user/foo" + cat file | curl -n -F "file=@-;filename=stdin" Client: Development (git): http://git.server-speed.net/users/flo/fb -- cgit v1.2.3-24-g4f1b From b88bdf6727a91451450a066f37e865e3b1b5a60d Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Thu, 29 Aug 2013 17:56:21 +0200 Subject: Replace echo with show_error; misc cleanup Signed-off-by: Florian Pritz --- application/controllers/file.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) (limited to 'application') diff --git a/application/controllers/file.php b/application/controllers/file.php index c336db92b..992252ff5 100644 --- a/application/controllers/file.php +++ b/application/controllers/file.php @@ -562,17 +562,14 @@ class File extends MY_Controller { $this->muser->require_access("apikey"); if (!is_cli_client()) { - echo "Not a listed cli client, please use the history to delete uploads.\n"; - return; + show_error("Not a listed cli client, please use the history to delete uploads.\n", 403); } $id = $this->uri->segment(3); $this->data["id"] = $id; if ($id && !$this->mfile->id_exists($id)) { - $this->output->set_status_header(404); - echo "Unknown ID '$id'.\n"; - return; + show_error("Unknown ID '$id'.", 404); } if ($this->mfile->delete_id($id)) { @@ -586,6 +583,7 @@ class File extends MY_Controller { function do_paste() { // desktop clients get a cookie to claim the ID later + // don't force them to log in just yet if (is_cli_client()) { $this->muser->require_access(); } @@ -620,6 +618,7 @@ class File extends MY_Controller { function do_upload() { // desktop clients get a cookie to claim the ID later + // don't force them to log in just yet if (is_cli_client()) { $this->muser->require_access("apikey"); } @@ -651,15 +650,15 @@ class File extends MY_Controller { UPLOAD_ERR_EXTENSION => "A PHP extension stopped the file upload", ); - $this->data["msg"] = "Unknown error."; + $msg = "Unknown error."; if (isset($errors[$file['error']])) { - $this->data["msg"] = $errors[$file['error']]; + $msg = $errors[$file['error']]; } else { - $this->data["msg"] = "Unknown error code: ".$file['error'].". Please report a bug."; + $msg = "Unknown error code: ".$file['error'].". Please report a bug."; } - show_error("Error while uploading: ".$this->data["msg"], 400); + show_error("Error while uploading: ".$msg, 400); } $filesize = filesize($file['tmp_name']); -- cgit v1.2.3-24-g4f1b From 39eacae51f4fbc239a11b150b752a76bccf74445 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Thu, 29 Aug 2013 17:57:09 +0200 Subject: claim_id: Fix error when called directly without last_upload data Signed-off-by: Florian Pritz --- application/controllers/file.php | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'application') diff --git a/application/controllers/file.php b/application/controllers/file.php index 992252ff5..6e660b306 100644 --- a/application/controllers/file.php +++ b/application/controllers/file.php @@ -704,9 +704,16 @@ class File extends MY_Controller { $this->muser->require_access(); $last_upload = $this->session->userdata("last_upload"); + + if ($last_upload === false) { + show_error("Failed to get last upload data"); + } + $ids = $last_upload["ids"]; $errors = array(); + assert(is_array($ids)); + foreach ($ids as $key => $id) { $filedata = $this->mfile->get_filedata($id); -- cgit v1.2.3-24-g4f1b From 5638bcbd816540ba1f20b75ccc1220f98028bbc3 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Thu, 29 Aug 2013 17:57:56 +0200 Subject: mfile/delete_id: Remove require_access call We expect the controller to take care of that, no need to double check. Signed-off-by: Florian Pritz --- application/models/mfile.php | 1 - 1 file changed, 1 deletion(-) (limited to 'application') diff --git a/application/models/mfile.php b/application/models/mfile.php index fe762d954..e862f1930 100644 --- a/application/models/mfile.php +++ b/application/models/mfile.php @@ -312,7 +312,6 @@ class Mfile extends CI_Model { function delete_id($id) { - $this->muser->require_access("apikey"); $filedata = $this->get_filedata($id); $userid = $this->muser->get_userid(); -- cgit v1.2.3-24-g4f1b From 752c59413b4899b295a9359eaef98dc9efb01533 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Thu, 29 Aug 2013 18:01:24 +0200 Subject: Add GET parameter for json output Signed-off-by: Florian Pritz --- application/core/MY_Controller.php | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'application') diff --git a/application/core/MY_Controller.php b/application/core/MY_Controller.php index 09b813b71..4c0fa278c 100644 --- a/application/core/MY_Controller.php +++ b/application/core/MY_Controller.php @@ -32,12 +32,18 @@ class MY_Controller extends CI_Controller { mb_internal_encoding('UTF-8'); $this->load->helper(array('form', 'filebin')); + // TODO: proper accept header handling or is this enough? if (isset($_SERVER["HTTP_ACCEPT"])) { if ($_SERVER["HTTP_ACCEPT"] == "application/json") { request_type("json"); } } + // Allow for easier testing in browser + if ($this->input->get("json") !== false) { + request_type("json"); + } + if (request_type() == "json" && ! in_array($this->uri->rsegment(2), $this->json_enabled_functions)) { show_error("Function not JSON enabled"); } -- cgit v1.2.3-24-g4f1b From eafc10e06fc0e08df684722e6ca2a221aebdf4d0 Mon Sep 17 00:00:00 2001 From: Florian Pritz Date: Sun, 1 Sep 2013 00:04:23 +0200 Subject: Disable CSRF checks for CLI requests Otherwise we get an error in the Security class trying to access $_SERVER["REQUEST_METHOD"]. Signed-off-by: Florian Pritz --- application/core/MY_Controller.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'application') diff --git a/application/core/MY_Controller.php b/application/core/MY_Controller.php index 4c0fa278c..312b0f763 100644 --- a/application/core/MY_Controller.php +++ b/application/core/MY_Controller.php @@ -85,7 +85,7 @@ class MY_Controller extends CI_Controller { $csrf_protection = false; } - if ($csrf_protection) { + if ($csrf_protection && !$this->input->is_cli_request()) { // 2 functions for accessing config options, really? $this->config->set_item('csrf_protection', true); config_item("csrf_protection", true); -- cgit v1.2.3-24-g4f1b From 69e0cba93445496e7b045b54ecefe8243276fd50 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sun, 1 Sep 2013 22:51:28 +0200 Subject: Autofocus username text box on upload_form Signed-off-by: Florian Pritz --- application/views/file/upload_form.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'application') diff --git a/application/views/file/upload_form.php b/application/views/file/upload_form.php index 841ac3746..21a2cc4e6 100644 --- a/application/views/file/upload_form.php +++ b/application/views/file/upload_form.php @@ -46,7 +46,7 @@ - + -- cgit v1.2.3-24-g4f1b