SECURITY: fixes users being able to re-run migrations

Author: lovasoa

CHANGELOG.md

@@ -1,5 +1,8 @@
 # CHANGELOG.md
 
+## 0.19.1 (2024-02-28)
+  - **SECURITY**: fixes users being able to re-run migrations by visiting `/sqlpage/migrations/NNNN_name.sql` pages. If you are using sqlpage migrations, your migrations are not idempotent, and you use the default SQLPAGE_WEB_ROOT (`./`) and `SQLPAGE_CONFIGURATION_DIRECTORY` (`./sqlpage/`), you should upgrade to this version as soon as possible. If you are using a custom `SQLPAGE_WEB_ROOT` or `SQLPAGE_CONFIGURATION_DIRECTORY` or your migrations are idempotent, you can upgrade at your convenience.
+
 ## 0.19.0 (2024-02-25)
 
  - Updated the chart component to use the latest version of the charting library

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "sqlpage"
-version = "0.19.0"
+version = "0.19.1"
 edition = "2021"
 description = "A SQL-only web application framework. Takes .sql files and formats the query result using pre-made configurable professional-looking components."
 keywords = ["web", "sql", "framework"]

Cargo.toml

@@ -18,7 +18,7 @@ Here are the available configuration options and their default values:
 | `database_connection_acquire_timeout_seconds` | 10                                                          | How long to wait when acquiring a database connection from the pool before giving up and returning an error.                                                                                                                                           |
 | `sqlite_extensions`                           |                                                             | An array of SQLite extensions to load, such as `mod_spatialite`                                                                                                                                                                                        |
 | `web_root`                                    | `.`                                                         | The root directory of the web server, where the `index.sql` file is located.                                                                                                                                                                           |
-| `configuration_directory`                     | `./sqlpage/`                                                | The directory where the `sqlpage.json` file is located. This is used to find the path to `templates/`, `migrations/`, and `on_connect.sql`. Obviously, this configuration parameter can be set only through environment variables, not through the `sqlpage.json` file itself in order to find the `sqlpage.json` file. |
+| `configuration_directory`                     | `./sqlpage/`                                                | The directory where the `sqlpage.json` file is located. This is used to find the path to `templates/`, `migrations/`, and `on_connect.sql`. Obviously, this configuration parameter can be set only through environment variables, not through the `sqlpage.json` file itself in order to find the `sqlpage.json` file. Be careful not to use a path that is accessible from the public WEB_ROOT |
 | `allow_exec`                                  | false                                                       | Allow usage of the `sqlpage.exec` function. Do this only if all users with write access to sqlpage query files and to the optional `sqlpage_files` table on the database are trusted.                                                                  |
 | `max_uploaded_file_size`                      | 5242880                                                     | Maximum size of uploaded files in bytes. Defaults to 5 MiB.                                                                                                                                                                                            |
 | `https_domain`                                |                                                             | Domain name to request a certificate for. Setting this parameter will automatically make SQLPage listen on port 443 and request an SSL certificate. The server will take a little bit longer to start the first time it has to request a certificate.  |

configuration.md

@@ -95,7 +95,16 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
         self.static_files.insert(path, Cached::new(contents));
     }
 
+    /// Gets a file from the cache, or loads it from the file system if it's not there
+    /// This is a privileged operation; it should not be used for user-provided paths
     pub async fn get(&self, app_state: &AppState, path: &PathBuf) -> anyhow::Result<Arc<T>> {
+        self.get_with_privilege(app_state, path, true).await
+    }
+    
+    /// Gets a file from the cache, or loads it from the file system if it's not there
+    /// The privileged parameter is used to determine whether the access should be denied
+    /// if the file is in the sqlpage/ config directory
+    pub async fn get_with_privilege(&self, app_state: &AppState, path: &PathBuf, privileged: bool) -> anyhow::Result<Arc<T>> {
         log::trace!("Attempting to get from cache {:?}", path);
         if let Some(cached) = self.cache.read().await.get(path) {
             if app_state.config.environment.is_prod() && !cached.needs_check() {
@@ -104,7 +113,7 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
             }
             match app_state
                 .file_system
-                .modified_since(app_state, path, cached.last_check_time(), true)
+                .modified_since(app_state, path, cached.last_check_time(), privileged)
                 .await
             {
                 Ok(false) => {
@@ -120,7 +129,7 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
         log::trace!("Loading and parsing {:?}", path);
         let file_contents = app_state
             .file_system
-            .read_to_string(app_state, path, true)
+            .read_to_string(app_state, path, privileged)
             .await;
 
         let parsed = match file_contents {

src/file_cache.rs

@@ -360,7 +360,7 @@ async fn process_sql_request(
     let app_state: &web::Data<AppState> = req.app_data().expect("app_state");
     let sql_file = app_state
         .sql_file_cache
-        .get(app_state, &sql_path)
+        .get_with_privilege(app_state, &sql_path, false)
         .await
         .with_context(|| format!("Unable to get SQL file {sql_path:?}"))
         .map_err(anyhow_err_to_actix)?;

src/webserver/http.rs

@@ -184,6 +184,20 @@ async fn test_csv_upload() -> actix_web::Result<()> {
     Ok(())
 }
 
+#[actix_web::test]
+/// `/sqlpage/migrations/0001_init.sql` should return a 403 Forbidden
+async fn privileged_paths_are_not_accessible() {
+    let resp_result = req_path("/sqlpage/migrations/0001_init.sql").await;
+    assert!(resp_result.is_err(), "Accessing a migration file should be forbidden");
+    let resp = resp_result.unwrap_err().error_response();
+    assert_eq!(resp.status(), http::StatusCode::FORBIDDEN);
+    assert!(
+        String::from_utf8_lossy(&resp.into_body().try_into_bytes().unwrap())
+            .to_lowercase()
+            .contains("forbidden"),
+    );
+}
+
 async fn get_request_to(path: &str) -> actix_web::Result<TestRequest> {
     let data = make_app_data().await;
     Ok(test::TestRequest::get().uri(path).app_data(data))