tursodatabase / tursodatabase/libsql

Unsound api: row.next() invalidation not covered by lifetime and may* cause use-after-free

Open
#2,257 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
17.2k
Forks
531
Avg merge
1h 12m
Merged PRs (30d)
1

Description

synopsis:

charaverk | ~/repos/libsql_execute/src ~$ cat main.rs 
fn main() {
    let runtime = tokio::runtime::Runtime::new().unwrap();
    runtime.block_on(async{
        let db = libsql::Builder::new_local("/tmp/test.db").build().await.unwrap();
        let conn = db.connect().unwrap();
/*
        conn.execute("CREATE TABLE IF NOT EXISTS test_table(id INTEGER PRIMARY KEY, data TEXT)", ()).await.unwrap();
        let _res = conn.execute(
            "INSERT INTO test_table(id, data) VALUES(0, ?1);
            ", [ "some str content" ]).await;
*/
        let mut res = conn.query(
            "SELECT data FROM test_table WHERE id == ?1;
            ", [ 0 ]).await.unwrap();
        let row = res.next().await.unwrap().unwrap();
        let str_ref = row.get_str(0).unwrap();
        let erm = res.next().await;
        //* unsound here, quickly debugged what cause, but still...
        let str_ = str_ref.to_owned(); //? invalidation that can lead to 'use after free' 
                                       //?   or just logical error?
        println!("{erm:?} {str_}");
    });
}

as in example, when from current row user get_str, it return non owning &str, and next call of rows.next() will cause invalidation of whatever was under reference str_ref

using previous row after call to next is already invalid, but it will not cause memory issues
using &str to previous row, probably not cause memory issues now:

@sivukhin says:

for local client libsql/sqlite reuse memory for the columns between next() calls which makes this bug possible...
I wouldn't rely on the reuse semantic too much. SQLite/libsql can free this memory after next() call too I think

so, seems like currently it can only lead to logical error, but would be better to somehow cover it, tho, i not know how to do so without allocating for each str

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Rust row API entry points row.get_str() and rows.next(), then reproduce the supplied local-client example. Determine the intended lifetime and invalidation contract for borrowed strings across next() calls, and define a test or documentation outcome that demonstrates the unsafe use is prevented or clearly constrained.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sqlite
Domain
backend-api-design, databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.