CVE-2021-37625: Unchecked Return Value (CWE-252)
value: Skytable is an open source NoSQL database. In versions prior to 0.6.4 an incorrect check of return value of the accept function in the run-loop for a TCP socket/TLS socket/TCP+TLS multi-socket causes an early exit from the run loop that should continue infinitely unless terminated by a local user, effectively causing the whole database server to shut down. This has severe impact and can be used to easily cause DoS attacks without the need to use much bandwidth. The attack vectors include using an incomplete TLS connection for example by not providing the certificate for the connection and using a specially crafted TCP packet that triggers the application layer backoff algorithm.
CWE description: The product does not check the return value from a method or function, which can prevent it from detecting unexpected states and conditions.
CVSS v3 base score: 7.5
Published: 2021-08-05T18:15Z
Repository: https://github.com/skytable/skytable
Language: Rust
Files changed: CHANGELOG.md, server/src/dbnet/macros.rs, server/src/dbnet/mod.rs, server/src/dbnet/tcp.rs, server/src/dbnet/tls.rs
Commit date: 2021-08-05 02:52:16 -0700
Commit message: Ignore errors in run loop

This fixes CVE-2021-37625

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07dce31..8f7f770 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -114,10 +114,17 @@ All changes in this project will be noted in this file.
 - Panic on incorrect data type in `skyd`
 - `sky-bench` no longer affects your personal data because it creates a random temporary table
   under the `default` keyspace
 - Fix log output in `sky-bench` even if the `--json` flag was passed
 - Use flocks to enable auto release of pid file, even if process is forcefully terminated
+- Fixes [CVE-2021-37625](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-37625)
+
+## Version 0.6.4 [2021-08-05]
+
+### Fixes
+
+- Fixes [CVE-2021-37625](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-37625) (backport)
 
 ## Version 0.6.3 [2021-06-27]
 
 ### Additions
 
diff --git a/server/src/dbnet/macros.rs b/server/src/dbnet/macros.rs
new file mode 100644
index 0000000..64faab2
--- /dev/null
+++ b/server/src/dbnet/macros.rs
@@ -0,0 +1,34 @@
+/*
+ * Created on Thu Aug 05 2021
+ *
+ * This file is a part of Skytable
+ * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source
+ * NoSQL database written by Sayan Nandan ("the Author") with the
+ * vision to provide flexibility in data modelling without compromising
+ * on performance, queryability or scalability.
+ *
+ * Copyright (c) 2021, Sayan Nandan <ohsayan@outlook.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <https://www.gnu.org/licenses/>.
+ *
+*/
+
+macro_rules! skip_loop_err {
+    ($expr:expr) => {
+        match $expr {
+            Ok(ret) => ret,
+            Err(_) => continue,
+        }
+    };
+}
diff --git a/server/src/dbnet/mod.rs b/server/src/dbnet/mod.rs
index 936dee3..c15783e 100644
--- a/server/src/dbnet/mod.rs
+++ b/server/src/dbnet/mod.rs
@@ -50,10 +50,12 @@ use std::sync::Arc;
 use tls::SslListener;
 use tokio::net::TcpListener;
 use tokio::sync::Semaphore;
 use tokio::sync::{broadcast, mpsc};
 pub mod connection;
+#[macro_use]
+mod macros;
 mod tcp;
 mod tls;
 
 pub const MAXIMUM_CONNECTION_LIMIT: usize = 50000;
 
diff --git a/server/src/dbnet/tcp.rs b/server/src/dbnet/tcp.rs
index 59b97cb..a7b1395 100644
--- a/server/src/dbnet/tcp.rs
+++ b/server/src/dbnet/tcp.rs
@@ -101,11 +101,19 @@ impl Listener {
     pub async fn run(&mut self) -> TResult<()> {
         loop {
             // Take the permit first, but we won't use it right now
             // that's why we will forget it
             self.base.climit.acquire().await.unwrap().forget();
-            let stream = self.accept().await?;
+            /*
+             SECURITY: Ignore any errors that may arise in the accept
+             loop. If we apply the try operator here, we will immediately
+             terminate the run loop causing the entire server to go down.
+             Also, do not log any errors because many connection errors
+             can arise and it will flood the log and might also result
+             in a crash
+            */
+            let stream = skip_loop_err!(self.accept().await);
             let mut chandle = ConnectionHandler::new(
                 self.base.db.clone(),
                 Connection::new(stream),
                 self.base.climit.clone(),
                 Terminator::new(self.base.signal.subscribe()),
diff --git a/server/src/dbnet/tls.rs b/server/src/dbnet/tls.rs
index 77c6199..0727700 100644
--- a/server/src/dbnet/tls.rs
+++ b/server/src/dbnet/tls.rs
@@ -112,19 +112,28 @@ impl SslListener {
     pub async fn run(&mut self) -> TResult<()> {
         loop {
             // Take the permit first, but we won't use it right now
             // that's why we will forget it
             self.base.climit.acquire().await.unwrap().forget();
-            let stream = self.accept().await?;
+            /*
+             SECURITY: Ignore any errors that may arise in the accept
+             loop. If we apply the try operator here, we will immediately
+             terminate the run loop causing the entire server to go down.
+             Also, do not log any errors because many connection errors
+             can arise and it will flood the log and might also result
+             in a crash
+            */
+            let stream = skip_loop_err!(self.accept().await);
             let mut sslhandle = ConnectionHandler::new(
                 self.base.db.clone(),
                 Connection::new(stream),
                 self.base.climit.clone(),
                 Terminator::new(self.base.signal.subscribe()),
                 self.base.terminate_tx.clone(),
             );
             tokio::spawn(async move {
+                log::debug!("Spawned listener task");
                 if let Err(e) = sslhandle.run().await {
                     log::error!("Error: {}", e);
                 }
             });
         }

Vulnerable code:             let stream = self.accept().await?;
            let stream = self.accept().await?;
Fixed code: - Fixes [CVE-2021-37625](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-37625)

## Version 0.6.4 [2021-08-05]

### Fixes

- Fixes [CVE-2021-37625](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2021-37625) (backport)
/*
 * Created on Thu Aug 05 2021
 *
 * This file is a part of Skytable
 * Skytable (formerly known as TerrabaseDB or Skybase) is a free and open-source
 * NoSQL database written by Sayan Nandan ("the Author") with the
 * vision to provide flexibility in data modelling without compromising
 * on performance, queryability or scalability.
 *
 * Copyright (c) 2021, Sayan Nandan <ohsayan@outlook.com>
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License
 * along with this program. If not, see <https://www.gnu.org/licenses/>.
 *
*/

macro_rules! skip_loop_err {
    ($expr:expr) => {
        match $expr {
            Ok(ret) => ret,
            Err(_) => continue,
        }
    };
}
#[macro_use]
mod macros;
            /*
             SECURITY: Ignore any errors that may arise in the accept
             loop. If we apply the try operator here, we will immediately
             terminate the run loop causing the entire server to go down.
             Also, do not log any errors because many connection errors
             can arise and it will flood the log and might also result
             in a crash
            */
            let stream = skip_loop_err!(self.accept().await);
            /*
             SECURITY: Ignore any errors that may arise in the accept
             loop. If we apply the try operator here, we will immediately
             terminate the run loop causing the entire server to go down.
             Also, do not log any errors because many connection errors
             can arise and it will flood the log and might also result
             in a crash
            */
            let stream = skip_loop_err!(self.accept().await);
                log::debug!("Spawned listener task");