v1.1.7: deep security & reliability audit — 33 bugs fixed
CI / test (push) Has been cancelled
CI / verify (push) Has been cancelled
Clients CI / build-server (push) Has been cancelled
Clients CI / test-python (push) Has been cancelled
Clients CI / test-javascript (push) Has been cancelled
Clients CI / test-nim (push) Has been cancelled
Clients CI / test-rust (push) Has been cancelled
CI / test (push) Has been cancelled
CI / verify (push) Has been cancelled
Clients CI / build-server (push) Has been cancelled
Clients CI / test-python (push) Has been cancelled
Clients CI / test-javascript (push) Has been cancelled
Clients CI / test-nim (push) Has been cancelled
Clients CI / test-rust (push) Has been cancelled
Critical (5): - Reject empty JWT secret when authEnabled (server.nim) - Fix 2PC marking uncontacted participants as prepared/committed (disttxn.nim) - Fix Raft commit index calculation for even-sized clusters (raft.nim) - Fix REP/DISTTXN protocol auth bypass (server.nim) - Fix HTTP backup/restore path traversal (httpserver.nim) High (11): - Fix WAL write race with flush (lsm.nim) - Fix MVCC savepoint/rollback deep-copy writeSet (mvcc.nim) - Fix table mutation during deadlock iteration (mvcc.nim) - Fix LIMIT 0 returning all rows (executor.nim) - Fix COUNT(col) counting NULL values — 3 locations (executor.nim) - Fix EXISTS subquery lowering missing subqueryPlan (executor.nim) - Fix Raft appendEntries/applyCommitted array vs logical index (raft.nim) - Fix timing attacks on constantTimeCompare and SCRAM (auth.nim, scram.nim) - Fix B-tree leaf merge phantom separator key (btree.nim) - Fix SSL verifyPeer not applied to newContext (ssl.nim) - Fix sharding connectWithTimeout missing SO_ERROR check (sharding.nim) - Fix sync replication returning success on partial ack (replication.nim) - Fix WebSocket JWT expiration not validated (websocket.nim) Medium (13): - Fix writeSSTable partial file → tmp + atomic rename (lsm.nim) - Fix multi-CTE table loss (executor.nim) - Fix nl_to_sql DML restricted to superuser (executor.nim) - Fix unbounded plan cache — max 10000 (adaptive.nim) - Fix migration lock crash persistence — timestamp + stale detection (executor.nim) - Fix admin panel auth (httpserver.nim) - Fix MVCC unbounded txn tracking — prune in compactVersions (mvcc.nim) - Fix connection pool maxLifetime check (pool.nim) - Fix JWT JSON parser backslash escapes (auth.nim) - Fix substr(s, start) returning single char (udf.nim) - Fix loadSSTable minimum file-size check (lsm.nim) - Fix compaction mmap leak (compaction.nim) - Fix JSON injection in hybrid_search_filtered (executor.nim) Low (4): - Raft loadState logs error instead of silent discard - Replication healthCheck double-close fixed - Lexer readIdent double column counting fixed - WebSocket frame 32-bit overflow guard All 448 tests passing, 0 failures. Bump version to 1.1.7.
This commit is contained in:
@@ -25,7 +25,6 @@ import std/osproc
|
||||
import std/strutils
|
||||
import std/times
|
||||
import std/algorithm
|
||||
import std/parseopt
|
||||
import std/json
|
||||
import barabadb/storage/lsm
|
||||
|
||||
@@ -670,6 +669,8 @@ proc readBackupMeta*(input: string): JsonNode =
|
||||
# CLI Entry Point
|
||||
# =============================================================================
|
||||
when isMainModule:
|
||||
import std/parseopt
|
||||
|
||||
var
|
||||
command = ""
|
||||
dataDir = DEFAULT_DATA_DIR
|
||||
|
||||
@@ -142,8 +142,15 @@ proc prepare*(txn: DistributedTransaction): bool =
|
||||
acquire(txn.lock)
|
||||
if allOk:
|
||||
txn.state = dtsPrepared
|
||||
for nodeId, _ in txn.participants.mpairs:
|
||||
txn.participants[nodeId].prepared = true
|
||||
# Only mark successfully contacted nodes as prepared, not uncontacted ones
|
||||
for nodeId in preparedNodes:
|
||||
if txn.participants.hasKey(nodeId):
|
||||
txn.participants[nodeId].prepared = true
|
||||
# Flag participants without host/port as needing recovery
|
||||
for nodeId, p in txn.participants.mpairs:
|
||||
if p.host.len == 0 or p.port == 0:
|
||||
p.commitPending = true # Needs manual coordination
|
||||
echo "[WARN] 2PC participant ", nodeId, " has no host/port — marked for recovery"
|
||||
else:
|
||||
# Rollback already-prepared participants; track failures for recovery
|
||||
var rollbackFailed = false
|
||||
@@ -192,8 +199,15 @@ proc commit*(txn: DistributedTransaction): bool =
|
||||
acquire(txn.lock)
|
||||
if allOk:
|
||||
txn.state = dtsCommitted
|
||||
for nodeId, _ in txn.participants.mpairs:
|
||||
txn.participants[nodeId].committed = true
|
||||
# Only mark successfully contacted nodes as committed, not uncontacted ones
|
||||
for nodeId in committedNodes:
|
||||
if txn.participants.hasKey(nodeId):
|
||||
txn.participants[nodeId].committed = true
|
||||
# Flag participants without host/port as needing recovery
|
||||
for nodeId, p in txn.participants.mpairs:
|
||||
if not p.committed and not p.commitPending:
|
||||
p.commitPending = true
|
||||
echo "[WARN] 2PC participant ", nodeId, " not contacted — marked for recovery"
|
||||
elif committedNodes.len > 0:
|
||||
# Partial commit — mark committed, flag uncommitted for recovery
|
||||
txn.state = dtsCommitted
|
||||
|
||||
@@ -463,6 +463,10 @@ proc backupHandler(server: HttpServer): RequestHandler =
|
||||
let allDatabases = if body != nil and "all" in body: body["all"].getBool() else: false
|
||||
let dbName = if body != nil and "database" in body: body["database"].getStr() else: ""
|
||||
let outputFile = if body != nil and "output" in body: body["output"].getStr() else: "backup_" & $getTime().toUnix() & ".tar.gz"
|
||||
# Path traversal protection: reject paths with .. or absolute paths outside dataRoot
|
||||
if ".." in outputFile or outputFile.startsWith("/"):
|
||||
ctx.json(%*{"error": "Invalid output path: must be relative and not contain '..'"}, 400)
|
||||
return
|
||||
let compression = if body != nil and "level" in body: body["level"].getInt() else: 6
|
||||
try:
|
||||
var ok = false
|
||||
@@ -520,6 +524,10 @@ proc restoreHandler(server: HttpServer): RequestHandler =
|
||||
ctx.json(%*{"error": "Missing 'input' in request body"}, 400)
|
||||
return
|
||||
let inputFile = body["input"].getStr()
|
||||
# Path traversal protection: reject paths with .. or absolute paths
|
||||
if ".." in inputFile or inputFile.startsWith("/"):
|
||||
ctx.json(%*{"error": "Invalid input path: must be relative and not contain '..'"}, 400)
|
||||
return
|
||||
let allDatabases = if body != nil and "all" in body: body["all"].getBool() else: false
|
||||
let dbName = if body != nil and "database" in body: body["database"].getStr() else: ""
|
||||
let dataRoot = server.registry.dataRoot
|
||||
@@ -553,6 +561,10 @@ proc restoreHandler(server: HttpServer): RequestHandler =
|
||||
|
||||
proc adminHandler(server: HttpServer): RequestHandler =
|
||||
return proc(request: Request) {.gcsafe.} =
|
||||
{.cast(gcsafe).}:
|
||||
let ctx = newContext(request)
|
||||
if server.config.authEnabled and not server.checkAuth(request, ctx):
|
||||
return
|
||||
let html = """
|
||||
<!DOCTYPE html><html><head>
|
||||
<meta charset='utf-8'><meta name='viewport' content='width=device-width,initial-scale=1'>
|
||||
|
||||
@@ -3,6 +3,7 @@ import std/tables
|
||||
import std/locks
|
||||
import std/monotimes
|
||||
import std/sets
|
||||
import std/sequtils
|
||||
import deadlock
|
||||
|
||||
type
|
||||
@@ -196,7 +197,6 @@ proc write*(tm: TxnManager, txn: Transaction, key: string, value: seq[byte]): bo
|
||||
if victimId in tm.activeTxns:
|
||||
tm.activeTxns[victimId].state = tsAborted
|
||||
tm.activeTxns.del(victimId)
|
||||
# Keep the wait edge so subsequent transactions can detect cycles
|
||||
release(tm.lock)
|
||||
return false # write-write conflict with uncommitted txn
|
||||
|
||||
@@ -240,11 +240,14 @@ proc delete*(tm: TxnManager, txn: Transaction, key: string): bool =
|
||||
|
||||
# Timeout-based deadlock detection: abort stale transactions
|
||||
let now = getMonoTime().ticks()
|
||||
var toAbort: seq[TxnId] = @[]
|
||||
for otherId, otherTxn in tm.activeTxns:
|
||||
if otherId != txn.id and otherTxn.state == tsActive:
|
||||
if now - otherTxn.startTime > tm.txnTimeoutMs * 1_000_000:
|
||||
otherTxn.state = tsAborted
|
||||
tm.activeTxns.del(otherId)
|
||||
toAbort.add(otherId)
|
||||
for otherId in toAbort:
|
||||
tm.activeTxns[otherId].state = tsAborted
|
||||
tm.activeTxns.del(otherId)
|
||||
|
||||
# Check for write-write conflict against other active transactions' write sets
|
||||
for otherId, otherTxn in tm.activeTxns:
|
||||
@@ -331,6 +334,14 @@ proc commit*(tm: TxnManager, txn: Transaction): bool =
|
||||
|
||||
proc compactVersions(tm: TxnManager) =
|
||||
## Remove old overwritten versions that are no longer visible to any active transaction.
|
||||
## Also prune stale committed/aborted transaction IDs.
|
||||
|
||||
# Find the oldest active snapshot for pruning
|
||||
var oldestSnapshot: uint64 = high(uint64)
|
||||
for txnId, txn in tm.activeTxns:
|
||||
if txn.state == tsActive and uint64(txn.snapshotMaxTxn) < oldestSnapshot:
|
||||
oldestSnapshot = uint64(txn.snapshotMaxTxn)
|
||||
|
||||
for key, versions in tm.globalVersions.mpairs:
|
||||
if versions.len <= 3:
|
||||
continue
|
||||
@@ -356,6 +367,20 @@ proc compactVersions(tm: TxnManager) =
|
||||
newVersions.add(v)
|
||||
versions = newVersions
|
||||
|
||||
# Prune committed/aborted txn IDs older than oldest active snapshot
|
||||
if oldestSnapshot < high(uint64):
|
||||
var newCommitted = initHashSet[TxnId]()
|
||||
for id in tm.committedTxnsSet:
|
||||
if uint64(id) >= oldestSnapshot:
|
||||
newCommitted.incl(id)
|
||||
tm.committedTxnsSet = newCommitted
|
||||
var newAborted = initHashSet[TxnId]()
|
||||
for id in tm.abortedTxns:
|
||||
if uint64(id) >= oldestSnapshot:
|
||||
newAborted.incl(id)
|
||||
tm.abortedTxns = newAborted
|
||||
tm.committedTxns = tm.committedTxns.filterIt(uint64(it) >= oldestSnapshot)
|
||||
|
||||
proc abortTxn*(tm: TxnManager, txn: Transaction): bool =
|
||||
acquire(tm.lock)
|
||||
if txn.state != tsActive:
|
||||
@@ -369,7 +394,10 @@ proc abortTxn*(tm: TxnManager, txn: Transaction): bool =
|
||||
return true
|
||||
|
||||
proc savepoint*(tm: TxnManager, txn: Transaction) =
|
||||
txn.savepoints.add(txn.writeSet)
|
||||
var saved = initTable[string, VersionedRecord]()
|
||||
for k, v in txn.writeSet:
|
||||
saved[k] = v
|
||||
txn.savepoints.add(saved)
|
||||
|
||||
proc rollbackToSavepoint*(tm: TxnManager, txn: Transaction): bool =
|
||||
if txn.savepoints.len == 0:
|
||||
|
||||
+35
-22
@@ -2,7 +2,6 @@
|
||||
import std/tables
|
||||
import std/sets
|
||||
import std/deques
|
||||
import std/algorithm
|
||||
import std/random
|
||||
import std/monotimes
|
||||
import std/asyncdispatch
|
||||
@@ -134,7 +133,7 @@ proc loadState(node: RaftNode) =
|
||||
raise newException(IOError, "Incomplete Raft log data read")
|
||||
node.log[i] = LogEntry(term: term, index: index, command: cmd, data: data)
|
||||
except IOError, OSError:
|
||||
discard
|
||||
echo "[WARN] Failed to load Raft state from ", path, ": ", getCurrentExceptionMsg()
|
||||
s.close()
|
||||
|
||||
proc newRaftNode*(id: string, peers: seq[string], raftPort: int = 0,
|
||||
@@ -194,9 +193,10 @@ proc findLogEntryByIndex(node: RaftNode, index: uint64): int =
|
||||
|
||||
proc applyCommitted(node: RaftNode) =
|
||||
while node.lastApplied < node.commitIndex:
|
||||
let idx = int(node.lastApplied)
|
||||
if idx < node.log.len:
|
||||
let entry = node.log[idx]
|
||||
inc node.lastApplied
|
||||
let pos = node.findLogEntryByIndex(node.lastApplied)
|
||||
if pos >= 0:
|
||||
let entry = node.log[pos]
|
||||
# Handle distributed transaction commands
|
||||
if entry.command.startsWith("DISTTXN:"):
|
||||
let parts = entry.command.split(":")
|
||||
@@ -212,7 +212,6 @@ proc applyCommitted(node: RaftNode) =
|
||||
else:
|
||||
if node.applyCommand != nil:
|
||||
node.applyCommand(entry.command, entry.data)
|
||||
inc node.lastApplied
|
||||
|
||||
proc becomeFollower*(node: RaftNode, term: uint64) =
|
||||
node.state = rsFollower
|
||||
@@ -330,12 +329,15 @@ proc appendEntries*(node: RaftNode, peerId: string): RaftMessage =
|
||||
let nextIdx = node.nextIndex.getOrDefault(peerId, node.lastLogIndex + 1)
|
||||
let prevIdx = nextIdx - 1
|
||||
var prevTerm: uint64 = 0
|
||||
if prevIdx > 0 and prevIdx <= uint64(node.log.len):
|
||||
prevTerm = node.log[prevIdx - 1].term
|
||||
if prevIdx > 0:
|
||||
let prevPos = node.findLogEntryByIndex(prevIdx)
|
||||
if prevPos >= 0:
|
||||
prevTerm = node.log[prevPos].term
|
||||
|
||||
var entries: seq[LogEntry] = @[]
|
||||
if nextIdx > 0:
|
||||
for i in int(nextIdx - 1)..<node.log.len:
|
||||
let startPos = node.findLogEntryByIndex(nextIdx)
|
||||
if startPos >= 0:
|
||||
for i in startPos..<node.log.len:
|
||||
entries.add(node.log[i])
|
||||
|
||||
return RaftMessage(
|
||||
@@ -391,18 +393,29 @@ proc handleAppendReply*(node: RaftNode, peerId: string, reply: RaftMessage) =
|
||||
node.matchIndex[peerId] = reply.matchIdx
|
||||
node.nextIndex[peerId] = reply.matchIdx + 1
|
||||
|
||||
# Update commit index
|
||||
var matchIndices: seq[uint64] = @[node.lastLogIndex]
|
||||
for p, idx in node.matchIndex:
|
||||
matchIndices.add(idx)
|
||||
matchIndices.sort()
|
||||
|
||||
let medianIdx = matchIndices[(matchIndices.len - 1) div 2]
|
||||
if medianIdx > node.commitIndex:
|
||||
if medianIdx <= node.lastLogIndex and
|
||||
node.log[medianIdx - 1].term == node.currentTerm:
|
||||
node.commitIndex = medianIdx
|
||||
node.applyCommitted()
|
||||
# Update commit index using true majority calculation
|
||||
let majority = (node.peers.len + 1 + 1) div 2 # majority of cluster (peers + leader)
|
||||
var newCommitIdx = node.commitIndex
|
||||
|
||||
# Check each index from highest to current commitIndex+1
|
||||
for idx in countdown(int(node.lastLogIndex), int(node.commitIndex) + 1):
|
||||
if idx <= 0:
|
||||
break
|
||||
# Only commit entries from current term (Raft safety property)
|
||||
if uint64(idx) <= node.lastLogIndex and node.log[idx - 1].term == node.currentTerm:
|
||||
# Count how many nodes have replicated this index
|
||||
var count = 1 # Leader itself
|
||||
for peerId2, mIdx in node.matchIndex:
|
||||
if mIdx >= uint64(idx):
|
||||
inc count
|
||||
# If majority has replicated, this is the new commit index
|
||||
if count >= majority:
|
||||
newCommitIdx = uint64(idx)
|
||||
break
|
||||
|
||||
if newCommitIdx > node.commitIndex:
|
||||
node.commitIndex = newCommitIdx
|
||||
node.applyCommitted()
|
||||
else:
|
||||
if node.nextIndex[peerId] > 1:
|
||||
dec node.nextIndex[peerId]
|
||||
|
||||
@@ -150,8 +150,9 @@ proc writeLsn*(rm: ReplicationManager, data: seq[byte]): uint64 =
|
||||
rm.pendingAcks.del(lsn)
|
||||
release(rm.lock)
|
||||
if replicasToShip.len > 0 and ackCount < replicasToShip.len:
|
||||
when defined(debug):
|
||||
echo "Replication sync: only ", ackCount, "/", replicasToShip.len, " replicas acked for LSN ", lsn
|
||||
# Sync replication requires ALL replicas to ack — fail if any missed
|
||||
echo "[ERROR] Sync replication failed: only ", ackCount, "/", replicasToShip.len, " replicas acked for LSN ", lsn
|
||||
return 0 # Indicate failure to satisfy sync replication guarantee
|
||||
return lsn
|
||||
of rmSemiSync:
|
||||
if replicasToShip.len > 0:
|
||||
@@ -259,7 +260,6 @@ proc healthCheck*(rm: ReplicationManager) =
|
||||
if not connectWithTimeout(sock, replica.host, Port(replica.port), 1000):
|
||||
connected = false
|
||||
else:
|
||||
defer: sock.close()
|
||||
sock.send("PING\n")
|
||||
var response = ""
|
||||
try:
|
||||
@@ -271,7 +271,7 @@ proc healthCheck*(rm: ReplicationManager) =
|
||||
except:
|
||||
connected = false
|
||||
finally:
|
||||
sock.close()
|
||||
try: sock.close() except: discard
|
||||
|
||||
if not connected:
|
||||
acquire(rm.lock)
|
||||
|
||||
@@ -49,6 +49,12 @@ type
|
||||
activeConnectionsLock*: Lock
|
||||
|
||||
proc newServerWithRegistry*(config: BaraConfig, registry: DatabaseRegistry): Server =
|
||||
# CRITICAL: Reject empty JWT secret when auth is enabled
|
||||
if config.authEnabled and config.jwtSecret.len == 0:
|
||||
raise newException(ValueError,
|
||||
"Security error: authEnabled is true but jwtSecret is empty. " &
|
||||
"Set BARADB_JWT_SECRET environment variable or jwt_secret in baradb.json")
|
||||
|
||||
let dbInfo = getOrCreateDatabase(registry, "default")
|
||||
let db = dbInfo.db
|
||||
let ctx = cast[ExecutionContext](cast[pointer](dbInfo.ctx))
|
||||
@@ -369,6 +375,9 @@ proc handleClient(server: Server, client: AsyncSocket, clientId: int) {.async.}
|
||||
|
||||
# Detect text-based DISTTXN RPC (starts with "DISTTXN")
|
||||
if headerData.len >= 7 and headerData[0..6] == "DISTTXN":
|
||||
if not authenticated:
|
||||
await client.send("ERR auth required\n")
|
||||
continue
|
||||
var rest = headerData[7..^1]
|
||||
while '\n' notin rest:
|
||||
let more = await client.recvWithTimeout(1024, idleTimeout)
|
||||
@@ -405,6 +414,9 @@ proc handleClient(server: Server, client: AsyncSocket, clientId: int) {.async.}
|
||||
|
||||
# Detect replication data (starts with "REP ")
|
||||
if headerData.len >= 4 and headerData[0..3] == "REP ":
|
||||
if not authenticated:
|
||||
await client.send("ERR auth required\n")
|
||||
continue
|
||||
var rest = headerData[4..^1]
|
||||
while '\n' notin rest:
|
||||
let more = await client.recvWithTimeout(1024, idleTimeout)
|
||||
|
||||
@@ -5,6 +5,8 @@ import std/net
|
||||
import std/strutils
|
||||
import std/nativesockets
|
||||
import std/tables
|
||||
when defined(posix):
|
||||
import std/posix
|
||||
|
||||
type
|
||||
ShardStrategy* = enum
|
||||
@@ -153,6 +155,13 @@ proc connectWithTimeout(sock: Socket, host: string, port: Port, timeoutMs: int):
|
||||
var fds = @[sock.getFd]
|
||||
if selectWrite(fds, timeoutMs) <= 0:
|
||||
return false
|
||||
when defined(posix):
|
||||
# Verify connection actually succeeded via SO_ERROR
|
||||
var err: cint = 0
|
||||
var errLen = SockLen(sizeof(err))
|
||||
discard posix.getsockopt(sock.getFd, 1'i32, 4'i32, addr err, addr errLen)
|
||||
if err != 0:
|
||||
return false
|
||||
sock.getFd.setBlocking(true)
|
||||
return true
|
||||
|
||||
|
||||
@@ -6,6 +6,8 @@ import std/tables
|
||||
import std/base64
|
||||
import std/sets
|
||||
import std/nativesockets
|
||||
import std/times
|
||||
import std/json
|
||||
when defined(windows):
|
||||
from std/winlean import TCP_NODELAY
|
||||
else:
|
||||
@@ -105,6 +107,8 @@ proc decodeFrame(data: string): (WsFrame, int) =
|
||||
if uint64(data.len) < uint64(offset) + len:
|
||||
return (Wsframe(), 0)
|
||||
|
||||
if len > uint64(high(int) - 1):
|
||||
return (Wsframe(), 0)
|
||||
let plen = int(len)
|
||||
if frame.masked:
|
||||
for i in 0..<plen:
|
||||
@@ -294,6 +298,13 @@ proc handleConnection(server: WsServer, client: AsyncSocket) {.async.} =
|
||||
await client.send("HTTP/1.1 401 Unauthorized\r\n\r\n")
|
||||
client.close()
|
||||
return
|
||||
# Validate JWT expiration
|
||||
if "exp" in token.claims:
|
||||
let exp = token.claims["exp"].node.getInt()
|
||||
if exp > 0 and epochTime().int64 > exp:
|
||||
await client.send("HTTP/1.1 401 Unauthorized\r\n\r\n")
|
||||
client.close()
|
||||
return
|
||||
except:
|
||||
await client.send("HTTP/1.1 401 Unauthorized\r\n\r\n")
|
||||
client.close()
|
||||
|
||||
Reference in New Issue
Block a user