Skip to content

Commit f7ea47e

Browse files
authored
Merge pull request #663 from jamescater2/fix/prepare-v2-pztail-bounds-check
Bounds-check pzTail in prepare_v2/v3 span overloads (fix AOOR variant of #430)
2 parents 870678c + 854eeb0 commit f7ea47e

8 files changed

Lines changed: 129 additions & 89 deletions

File tree

src/SQLitePCLRaw.provider.e_sqlite3/Generated/provider_e_sqlite3_funcptrs_notwin.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -270,11 +270,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v2(sqlite3 db, ReadOnlySpan<byte> sq
270270
fixed (byte* p_sql = sql)
271271
{
272272
var rc = NativeMethods.sqlite3_prepare_v2(db, p_sql, sql.Length, out stm, out var p_tail);
273-
var len_consumed = (int) (p_tail - p_sql);
274-
int len_remain = sql.Length - len_consumed;
275-
if (len_remain > 0)
273+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
276274
{
277-
tail = sql.Slice(len_consumed, len_remain);
275+
var len_consumed = (int) (p_tail - p_sql);
276+
int len_remain = sql.Length - len_consumed;
277+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
278278
}
279279
else
280280
{
@@ -301,11 +301,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v3(sqlite3 db, ReadOnlySpan<byte> sq
301301
fixed (byte* p_sql = sql)
302302
{
303303
var rc = NativeMethods.sqlite3_prepare_v3(db, p_sql, sql.Length, flags, out stm, out var p_tail);
304-
var len_consumed = (int) (p_tail - p_sql);
305-
int len_remain = sql.Length - len_consumed;
306-
if (len_remain > 0)
304+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
307305
{
308-
tail = sql.Slice(len_consumed, len_remain);
306+
var len_consumed = (int) (p_tail - p_sql);
307+
int len_remain = sql.Length - len_consumed;
308+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
309309
}
310310
else
311311
{

src/SQLitePCLRaw.provider.e_sqlite3/Generated/provider_e_sqlite3_funcptrs_win.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -273,11 +273,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v2(sqlite3 db, ReadOnlySpan<byte> sq
273273
fixed (byte* p_sql = sql)
274274
{
275275
var rc = NativeMethods.sqlite3_prepare_v2(db, p_sql, sql.Length, out stm, out var p_tail);
276-
var len_consumed = (int) (p_tail - p_sql);
277-
int len_remain = sql.Length - len_consumed;
278-
if (len_remain > 0)
276+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
279277
{
280-
tail = sql.Slice(len_consumed, len_remain);
278+
var len_consumed = (int) (p_tail - p_sql);
279+
int len_remain = sql.Length - len_consumed;
280+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
281281
}
282282
else
283283
{

src/SQLitePCLRaw.provider.e_sqlite3/Generated/provider_e_sqlite3_prenet5_notwin.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -269,11 +269,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v2(sqlite3 db, ReadOnlySpan<byte> sq
269269
fixed (byte* p_sql = sql)
270270
{
271271
var rc = NativeMethods.sqlite3_prepare_v2(db, p_sql, sql.Length, out stm, out var p_tail);
272-
var len_consumed = (int) (p_tail - p_sql);
273-
int len_remain = sql.Length - len_consumed;
274-
if (len_remain > 0)
272+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
275273
{
276-
tail = sql.Slice(len_consumed, len_remain);
274+
var len_consumed = (int) (p_tail - p_sql);
275+
int len_remain = sql.Length - len_consumed;
276+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
277277
}
278278
else
279279
{
@@ -300,11 +300,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v3(sqlite3 db, ReadOnlySpan<byte> sq
300300
fixed (byte* p_sql = sql)
301301
{
302302
var rc = NativeMethods.sqlite3_prepare_v3(db, p_sql, sql.Length, flags, out stm, out var p_tail);
303-
var len_consumed = (int) (p_tail - p_sql);
304-
int len_remain = sql.Length - len_consumed;
305-
if (len_remain > 0)
303+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
306304
{
307-
tail = sql.Slice(len_consumed, len_remain);
305+
var len_consumed = (int) (p_tail - p_sql);
306+
int len_remain = sql.Length - len_consumed;
307+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
308308
}
309309
else
310310
{

src/SQLitePCLRaw.provider.e_sqlite3/Generated/provider_e_sqlite3_prenet5_win.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -272,11 +272,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v2(sqlite3 db, ReadOnlySpan<byte> sq
272272
fixed (byte* p_sql = sql)
273273
{
274274
var rc = NativeMethods.sqlite3_prepare_v2(db, p_sql, sql.Length, out stm, out var p_tail);
275-
var len_consumed = (int) (p_tail - p_sql);
276-
int len_remain = sql.Length - len_consumed;
277-
if (len_remain > 0)
275+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
278276
{
279-
tail = sql.Slice(len_consumed, len_remain);
277+
var len_consumed = (int) (p_tail - p_sql);
278+
int len_remain = sql.Length - len_consumed;
279+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
280280
}
281281
else
282282
{
@@ -303,11 +303,11 @@ unsafe int ISQLite3Provider.sqlite3_prepare_v3(sqlite3 db, ReadOnlySpan<byte> sq
303303
fixed (byte* p_sql = sql)
304304
{
305305
var rc = NativeMethods.sqlite3_prepare_v3(db, p_sql, sql.Length, flags, out stm, out var p_tail);
306-
var len_consumed = (int) (p_tail - p_sql);
307-
int len_remain = sql.Length - len_consumed;
308-
if (len_remain > 0)
306+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
309307
{
310-
tail = sql.Slice(len_consumed, len_remain);
308+
var len_consumed = (int) (p_tail - p_sql);
309+
int len_remain = sql.Length - len_consumed;
310+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
311311
}
312312
else
313313
{

src/common/tests_xunit.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,90 @@ public void test_prepare_tail_string()
349349
}
350350
}
351351

352+
// Regression test for the ArgumentOutOfRangeException variant of the
353+
// long-standing prepare-race bug class (see issues #108, #321, #430,
354+
// #479, #588). The provider's span overload of sqlite3_prepare_v2
355+
// computes `(int)(p_tail - p_sql)` and uses it as a Slice start.
356+
//
357+
// On SQLITE_MISUSE paths (unsafe db handle, null zSql), native returns
358+
// without writing *pzTail, leaving the caller's `out byte* p_tail`
359+
// local at its .locals-init default of 0. For a non-empty SQL input,
360+
// p_sql is a real non-null pointer, so `p_tail - p_sql` is a 64-bit
361+
// signed difference that truncates to an int whose sign depends on
362+
// bit 31 of the low 32 bits of p_sql. When bit 31 is clear (the sql
363+
// buffer sits at a typical high managed-heap address on x64), the
364+
// cast produces a large negative int, and `sql.Slice(negative, ...)`
365+
// throws ArgumentOutOfRangeException instead of cleanly returning
366+
// the error rc to the caller.
367+
//
368+
// We trigger the unsafe-db path deterministically via manual_close_v2,
369+
// and we force the sql buffer into the bit-31-clear address range by
370+
// growing the heap with a rolling allocator before each iteration.
371+
// On a fresh process the heap can start either side of 2^31, so
372+
// without the rolling growth the test is stochastic. With it, every
373+
// iteration fires the bug on unpatched builds.
374+
//
375+
// The patched provider bounds-checks p_tail against
376+
// [p_sql, p_sql + sql.Length] before computing the slice, so this
377+
// returns rc=SQLITE_MISUSE + tail=Empty instead of throwing.
378+
[Fact]
379+
public void test_prepare_v2_span_tolerates_uninitialised_pzTail()
380+
{
381+
var rolling = new byte[256][];
382+
var rnd = new Random(42);
383+
for (var i = 0; i < 512; i++)
384+
{
385+
rolling[i % rolling.Length] = new byte[rnd.Next(1024, 128 * 1024)];
386+
387+
var db = ugly.open(":memory:");
388+
db.manual_close_v2();
389+
try
390+
{
391+
var sql = u.to_utf8("SELECT 1;");
392+
393+
int rc = raw.sqlite3_prepare_v2(db, sql.AsSpan(), out var stmt, out var tail);
394+
395+
Assert.Equal(raw.SQLITE_MISUSE, rc);
396+
Assert.True(tail.IsEmpty);
397+
stmt?.Dispose();
398+
}
399+
finally
400+
{
401+
db.Dispose();
402+
}
403+
}
404+
GC.KeepAlive(rolling);
405+
}
406+
407+
[Fact]
408+
public void test_prepare_v3_span_tolerates_uninitialised_pzTail()
409+
{
410+
var rolling = new byte[256][];
411+
var rnd = new Random(42);
412+
for (var i = 0; i < 512; i++)
413+
{
414+
rolling[i % rolling.Length] = new byte[rnd.Next(1024, 128 * 1024)];
415+
416+
var db = ugly.open(":memory:");
417+
db.manual_close_v2();
418+
try
419+
{
420+
var sql = u.to_utf8("SELECT 1;");
421+
422+
int rc = raw.sqlite3_prepare_v3(db, sql.AsSpan(), 0, out var stmt, out var tail);
423+
424+
Assert.Equal(raw.SQLITE_MISUSE, rc);
425+
Assert.True(tail.IsEmpty);
426+
stmt?.Dispose();
427+
}
428+
finally
429+
{
430+
db.Dispose();
431+
}
432+
}
433+
GC.KeepAlive(rolling);
434+
}
435+
352436
[Fact]
353437
public void test_bind_parameter_index()
354438
{

src/providers/provider.tt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -528,11 +528,11 @@ namespace SQLitePCL
528528
fixed (byte* p_sql = sql)
529529
{
530530
var rc = NativeMethods.sqlite3_prepare_v2(db, p_sql, sql.Length, out stm, out var p_tail);
531-
var len_consumed = (int) (p_tail - p_sql);
532-
int len_remain = sql.Length - len_consumed;
533-
if (len_remain > 0)
531+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
534532
{
535-
tail = sql.Slice(len_consumed, len_remain);
533+
var len_consumed = (int) (p_tail - p_sql);
534+
int len_remain = sql.Length - len_consumed;
535+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
536536
}
537537
else
538538
{
@@ -559,11 +559,11 @@ namespace SQLitePCL
559559
fixed (byte* p_sql = sql)
560560
{
561561
var rc = NativeMethods.sqlite3_prepare_v3(db, p_sql, sql.Length, flags, out stm, out var p_tail);
562-
var len_consumed = (int) (p_tail - p_sql);
563-
int len_remain = sql.Length - len_consumed;
564-
if (len_remain > 0)
562+
if (p_tail != null && p_tail >= p_sql && p_tail <= p_sql + sql.Length)
565563
{
566-
tail = sql.Slice(len_consumed, len_remain);
564+
var len_consumed = (int) (p_tail - p_sql);
565+
int len_remain = sql.Length - len_consumed;
566+
tail = len_remain > 0 ? sql.Slice(len_consumed, len_remain) : ReadOnlySpan<byte>.Empty;
567567
}
568568
else
569569
{

src/tests/my_batteries_v2.cs

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
/*
2-
Copyright 2014-2019 SourceGear, LLC
1+
/*
2+
Copyright 2014-2026 SourceGear, LLC
33
44
Licensed under the Apache License, Version 2.0 (the "License");
55
you may not use this file except in compliance with the License.
@@ -14,55 +14,13 @@ You may obtain a copy of the License at
1414
limitations under the License.
1515
*/
1616

17-
using System;
18-
using System.Collections.Generic;
19-
using System.Reflection;
20-
using System.IO;
21-
2217
namespace SQLitePCL
2318
{
2419
public static class Batteries_V2
2520
{
26-
class MyGetFunctionPointer : IGetFunctionPointer
27-
{
28-
readonly IntPtr _dll;
29-
public MyGetFunctionPointer(IntPtr dll)
30-
{
31-
_dll = dll;
32-
}
33-
34-
public IntPtr GetFunctionPointer(string name)
35-
{
36-
if (NativeLibrary.TryGetExport(_dll, name, out var f))
37-
{
38-
//System.Console.WriteLine("{0}.{1} : {2}", _dll, name, f);
39-
return f;
40-
}
41-
else
42-
{
43-
return IntPtr.Zero;
44-
}
45-
}
46-
}
47-
static IGetFunctionPointer MakeDynamic(string name, int flags)
48-
{
49-
// TODO should this be GetExecutingAssembly()?
50-
var assy = typeof(SQLitePCL.raw).Assembly;
51-
var dll = SQLitePCL.NativeLibrary.Load(name, assy, flags);
52-
var gf = new MyGetFunctionPointer(dll);
53-
return gf;
54-
}
55-
static void DoDynamic_cdecl(string name, int flags)
56-
{
57-
var gf = MakeDynamic(name, flags);
58-
SQLitePCL.SQLite3Provider_dynamic_cdecl.Setup(name, gf);
59-
SQLitePCL.raw.SetProvider(new SQLite3Provider_dynamic_cdecl());
60-
}
61-
6221
public static void Init()
6322
{
64-
DoDynamic_cdecl("e_sqlite3", NativeLibrary.WHERE_PLAIN);
23+
raw.SetProvider(new SQLite3Provider_e_sqlite3());
6524
}
6625
}
6726
}
68-

src/tests/tests.csproj

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,17 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22
<PropertyGroup>
3-
<TargetFramework>netcoreapp3.1</TargetFramework>
3+
<TargetFramework>$(tfm_net)</TargetFramework>
44
<EnableDefaultCompileItems>false</EnableDefaultCompileItems>
55
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
6+
<IsPackable>false</IsPackable>
67
</PropertyGroup>
78
<ItemGroup>
8-
<ProjectReference Include="..\SQLitePCLRaw.core\SQLitePCLRaw.core.csproj" />
9-
<ProjectReference Include="..\SQLitePCLRaw.provider.dynamic_cdecl\SQLitePCLRaw.provider.dynamic_cdecl.csproj" />
10-
<ProjectReference Include="..\SQLitePCLRaw.nativelibrary\SQLitePCLRaw.nativelibrary.csproj" />
11-
<!-- <ProjectReference Include="..\SQLitePCLRaw.batteries_v2.e_sqlite3.dynamic\SQLitePCLRaw.batteries_v2.e_sqlite3.dynamic.csproj" /> -->
12-
<ProjectReference Include="..\SQLitePCLRaw.ugly\SQLitePCLRaw.ugly.csproj" />
9+
<ProjectReference Include="..\SQLitePCLRaw.core\SQLitePCLRaw.core.csproj" />
10+
<ProjectReference Include="..\SQLitePCLRaw.provider.e_sqlite3\SQLitePCLRaw.provider.e_sqlite3.csproj" />
11+
<ProjectReference Include="..\SQLitePCLRaw.ugly\SQLitePCLRaw.ugly.csproj" />
1312
</ItemGroup>
1413
<ItemGroup>
15-
<PackageReference Include="AltCover" Version="5.3.675" />
16-
<DotNetCliToolReference Include="dotnet-reportgenerator-cli" Version="4.1.10" />
14+
<PackageReference Include="SourceGear.sqlite3" Version="$(lib_e_sqlite3_package_reference_version)" />
1715
<PackageReference Include="microsoft.net.test.sdk" Version="$(depversion_microsoft_net_test_sdk)" />
1816
<PackageReference Include="xunit" Version="$(depversion_xunit)" />
1917
<PackageReference Include="xunit.runner.visualstudio" Version="$(depversion_xunit_runner_visualstudio)" />

0 commit comments

Comments
 (0)