resilar / sqleet

SQLite3 encryption that sucks less
The Unlicense
375 stars 55 forks source link

Rekey example not working in 3.30.0 release #32

Closed jbuchman closed 4 years ago

jbuchman commented 4 years ago

I was updating my project's sqleet to 3.30.0 and noticed that the sqlite3_rekey function was no longer able to rekey my database. I was getting SQLITE_MISUSE back when attempting to rekey the database.

Dug around a bit and haven't been able to find the cause of the issue, but I have also found that the example in your readme no longer works:

buchmanj@Accio ~/D/sqleet-v0.30.0> hexdump -C test.sqlite
00000000  53 51 4c 69 74 65 20 66  6f 72 6d 61 74 20 33 00  |SQLite format 3.|
00000010  10 00 01 01 00 40 20 20  00 00 00 01 00 00 00 02  |.....@  ........|
00000020  00 00 00 00 00 00 00 00  00 00 00 01 00 00 00 04  |................|
00000030  00 00 00 00 00 00 00 00  00 00 00 01 00 00 00 00  |................|
00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000050  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 01  |................|
00000060  00 2e 3b f0 0d 00 00 00  01 0f d4 00 0f d4 00 00  |..;.............|
00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000fd0  00 00 00 00 2a 01 06 17  15 15 01 39 74 61 62 6c  |....*......9tabl|
00000fe0  65 74 65 73 74 74 65 73  74 02 43 52 45 41 54 45  |etesttest.CREATE|
00000ff0  20 54 41 42 4c 45 20 74  65 73 74 20 28 69 64 29  | TABLE test (id)|
00001000  0d 00 00 00 00 10 00 00  00 00 00 00 00 00 00 00  |................|
00001010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00002000
buchmanj@Accio ~/D/sqleet-v0.30.0> ./sqleet test.sqlite
SQLite version 3.30.0 2019-10-04 15:03:17
Enter ".help" for usage hints.
sqlite> PRAGMA rekey='swordfish';
sqlite> .quit
buchmanj@Accio ~/D/sqleet-v0.30.0> hexdump -C test.sqlite
00000000  53 51 4c 69 74 65 20 66  6f 72 6d 61 74 20 33 00  |SQLite format 3.|
00000010  10 00 01 01 00 40 20 20  00 00 00 01 00 00 00 02  |.....@  ........|
00000020  00 00 00 00 00 00 00 00  00 00 00 01 00 00 00 04  |................|
00000030  00 00 00 00 00 00 00 00  00 00 00 01 00 00 00 00  |................|
00000040  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000050  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 01  |................|
00000060  00 2e 3b f0 0d 00 00 00  01 0f d4 00 0f d4 00 00  |..;.............|
00000070  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00000fd0  00 00 00 00 2a 01 06 17  15 15 01 39 74 61 62 6c  |....*......9tabl|
00000fe0  65 74 65 73 74 74 65 73  74 02 43 52 45 41 54 45  |etesttest.CREATE|
00000ff0  20 54 41 42 4c 45 20 74  65 73 74 20 28 69 64 29  | TABLE test (id)|
00001000  0d 00 00 00 00 10 00 00  00 00 00 00 00 00 00 00  |................|
00001010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00002000
buchmanj@Accio ~/D/sqleet-v0.30.0>

Am I doing something wrong?

jbuchman commented 4 years ago

I also tested v0.29.0, v0.28.0, and v0.27.2

None of these worked as expected.

However, v0.27.1 does work as expected.

buchmanj@Accio ~/D/sqleet-v0.27.1> ./sqleet
SQLite version 3.27.1 2019-02-08 13:17:39
Enter ".help" for usage hints.
Connected to a transient in-memory database.
Use ".open FILENAME" to reopen on a persistent database.
sqlite> ^D
buchmanj@Accio ~/D/sqleet-v0.27.1> ./sqleet test.sqlite
SQLite version 3.27.1 2019-02-08 13:17:39
Enter ".help" for usage hints.
sqlite> PRAGMA rekey='swordfish';
sqlite> ^D
buchmanj@Accio ~/D/sqleet-v0.27.1> hexdump -C test.sqlite
00000000  14 c2 ed e3 36 fb f2 9c  27 75 7d 9d f8 6b 61 0a  |....6...'u}..ka.|
00000010  56 61 e3 62 7c 76 8f 30  12 54 74 d0 7b c4 fd 1e  |Va.b|v.0.Tt.{...|
00000020  f2 26 f3 c7 80 f3 8c 23  4f a5 ef e3 d4 8e 03 a7  |.&.....#O.......|
00000030  e8 c6 cf de 96 2c 33 70  2e 0b 5e b1 22 50 63 e9  |.....,3p..^."Pc.|
00000040  66 00 47 72 d9 39 e8 9a  38 a6 7d b7 4c 07 5e ad  |f.Gr.9..8.}.L.^.|
00000050  f4 b8 50 4e 5c 0d 21 d8  03 a2 82 c4 6e b3 1b 36  |..PN\.!.....n..6|
00000060  c6 76 52 fc 1f 84 c9 3e  2b 7b 55 cc 43 8f 13 d3  |.vR....>+{U.C...|
00000070  bc 41 8f 53 36 96 35 91  47 30 c3 78 ed c8 5d f7  |.A.S6.5.G0.x..].|
00000080  05 b2 82 84 55 cf 98 25  a3 32 5c aa f9 9f 36 dc  |....U..%.2\...6.|
00000090  c5 e6 8b 29 0d cb aa db  e8 1d d3 27 1c e6 11 af  |...).......'....|
000000a0  34 5c c7 04 7c f3 63 f5  ad 5b b5 5b 51 52 d8 59  |4\..|.c..[.[QR.Y|
jbuchman commented 4 years ago

I've tested this on both Mac, BSD and Linux based systems. It seems that it's only failing on Mac / iOS systems. I was not able to reproduce on a BSD or Linux box. It only fails on Mac/iOS systems.

nyneplus commented 4 years ago

I can confirm the rekey issue. Android jni and windows both do not work. Only 0.27.1 does.

resilar commented 4 years ago

Sorry for the delay, especially given the serious nature of the bug. Automated cross-platform tests are planned to be implemented prior to v1 in order to catch platform-specific issues like this in the future.

TL;DR: Fix in master. Please test and report back as I have yet to reproduce this issue myself. A fixed release sqleet v0.30.1 will be published once the fix is confirmed to resolve the issue.


I do not have Windows at hand for reproducing, but AddressSanitizer:heart: found an error introduced with URI API (shortly after 0.27.1) which likely leads to rekey failing in certain conditions. Rekey attaches a vacuum database that has no filename (URI) associated with it, which results in sqleet trying to parse URI configuration from an empty string zUri.

145 int codec_parse_uri_config(Codec *codec, const char *zUri)
146 {
147     int rc, pagesize;
148     const char *value;
149 
150     /* Override page_size PRAGMA */
151     pagesize = (int)sqlite3_uri_int64(zUri, "page_size", -1);

Line 151 calls sqlite3_uri_int64() with a zero-length URI, that is, zUri[0] = '\0'. Internally, zUri gets passed to sqlite3_uri_parameter() for parsing the value of the page_size URI parameter:

160417 SQLITE_API const char *sqlite3_uri_parameter(const char *zFilename, const char *zParam){
160418   if( zFilename==0 || zParam==0 ) return 0;
160419   zFilename += sqlite3Strlen30(zFilename) + 1;
160420   while( zFilename[0] ){

Upon entering the function, zFilename (i.e., the passed-in zUri) is a pointer to an empty string. Line 160419 increments the pointer by 1, thereby causing an out-of-bound access (zUri[1]) in the while condition in line 160420.

The fix is trivial: Set zUri to NULL if codec_parse_uri_config() receives an empty URL string. Interestingly, (internal) SQLite3 URI functions handle NULL URIs correctly, whereas empty URI strings produce undefined behavior.

Fixed in master commit 695bc40.

resilar commented 4 years ago

I was unable to reproduce the rekeying error using Win10 and msys2 gcc. Probably depends on the compiler used in addition to operating system. This is not surprising assuming the root cause is an out-of-bound memory access in heap (such bugs tend to be nondeterministic in nature).

However, the fact that the bug seems to be introduced between releases v0.27.1 and v0.27.2 makes me slightly worried. URI API was added in v0.30.0 so the bug described in my previous comment did not exist before v0.30.0. There also were no significant changes in sqleet or SQLite3 code between vesions v0.27.1 and v0.27.2 that could explain the rekey bug.

Nonetheless, let's see if the proposed fix addresses the rekey bug. If not, I will try to figure out some way to reproduce the issue in my test environment.


Must be noted that SQLITE_MISUSE is a very rare error code in SQLite3 code base and sqleet returns it only in the case of invalid URI configuration. Thus, SQLITE_MISUSE strongly suggests a problem in URI configuration parsing. Rekeying being broken before the introduction of URI API (v0.30.0) is unexpected.

resilar commented 4 years ago

No luck reproducing on Android in Termux environment with clang-compiled sqleet v0.30.0 (unfixed release). Oh well, hopefully someone who has been able to trigger the bug can test the fix in latest master branch.

nyneplus commented 4 years ago

I tested the fix on Android JNI 21 And it seems to work perfectly. Windows i can test but only POSIX compiled like clang/gcc.

resilar commented 4 years ago

That's great! Fixed sqleet v0.30.1 release is out now. I guess I have to figure out a systematic way to backport these kind of critical fixes to older releases as well.

If it turns out that the fix is incomplete and rekeying still fails in other environments, feel free to reopen this issue.

Thanks everybody for your contribution to sqleet.