Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

mailing lists
Hello,

I have seen that SQLite uses normally parameters of type "int" to pass the size of a variable (see sqlite3_bind_blob, fourth parameter). When compiling SQLite3 with Clang and some warnings enabled I get warnings when passing sizeof(...) as the fourth parameter. The reason is that sizeof(...) and other similar functions return the size as a size_t variable. And a size_t variable does not have to have the same size as an int variable.

Is it possible to change the fourth parameter in sqlite3_bind_XXX (and probably other locations) because this seems to be for me the appropriate type?! On iOS 64bit the size of int is 4 bytes and the size of size_t is 8 bytes. In this case the fourth parameter is actually not even able (theoretically) to store the length of a blob or text variable correctly.

Regards,
Hartwig

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Stephen Chrzanowski
Wouldn't it come down to the compiler you're using that'd indicate the
number of bytes associated to an integer type, or at least tell the
compiler to compile integer types to 64-bit?


On Sun, Sep 7, 2014 at 1:02 PM, skywind mailing lists <
[hidden email]> wrote:

> Hello,
>
> I have seen that SQLite uses normally parameters of type "int" to pass the
> size of a variable (see sqlite3_bind_blob, fourth parameter). When
> compiling SQLite3 with Clang and some warnings enabled I get warnings when
> passing sizeof(...) as the fourth parameter. The reason is that sizeof(...)
> and other similar functions return the size as a size_t variable. And a
> size_t variable does not have to have the same size as an int variable.
>
> Is it possible to change the fourth parameter in sqlite3_bind_XXX (and
> probably other locations) because this seems to be for me the appropriate
> type?! On iOS 64bit the size of int is 4 bytes and the size of size_t is 8
> bytes. In this case the fourth parameter is actually not even able
> (theoretically) to store the length of a blob or text variable correctly.
>
> Regards,
> Hartwig
>
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
>
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Stephan Beal-3
On Sun, Sep 7, 2014 at 8:08 PM, Stephen Chrzanowski <[hidden email]>
wrote:

> On Sun, Sep 7, 2014 at 1:02 PM, skywind mailing lists <
> [hidden email]> wrote:> type?! On iOS 64bit the size of int is 4
> bytes and the size of size_t is 8
>
> bytes. In this case the fourth parameter is actually not even able
> > (theoretically) to store the length of a blob or text variable correctly.
>

http://www.sqlite.org/c3ref/bind_blob.html

"In those routines that have a fourth argument, its value is the number of
bytes in the parameter. To be clear: the value is the number of *bytes* in
the value, not the number of characters. If the fourth parameter to
sqlite3_bind_text() or sqlite3_bind_text16() is negative, then the length
of the string is the number of bytes up to the first zero terminator."




i.e. changing them to size_t would change the semantics and break and and
all applications which rely on the current semantics (some of which are
mine).


See also:

http://www.sqlite.org/limits.html

which documents the 31-bit limit.


--
----- stephan beal
http://wanderinghorse.net/home/stephan/
http://gplus.to/sgbeal
"Freedom is sloppy. But since tyranny's the only guaranteed byproduct of
those who insist on a perfect world, freedom will have to do." -- Bigby Wolf
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Richard Hipp-3
In reply to this post by mailing lists
On Sun, Sep 7, 2014 at 1:02 PM, skywind mailing lists <
[hidden email]> wrote:

>
> Is it possible to change the fourth parameter in sqlite3_bind_XXX (and
> probably other locations) because this seems to be for me the appropriate
> type?!
>

No.  That would be a compatibility break.

Please use a cast to silence the compiler warnings.  "(int)sizeof(...)"
instead of just "sizeof(...)".

--
D. Richard Hipp
[hidden email]
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Roger Binns
In reply to this post by mailing lists
On 07/09/14 10:02, skywind mailing lists wrote:
> I have seen that SQLite uses normally parameters of type "int" to pass the size of a variable

Correct.  It should be using size_t or ssize_t, but the SQLite developers
chose not to do that, especially as at the time of the decision those
weren't always available types.

I have whined about this over the years, including showing that all open
source callers treated the parameter as though it was (s)size_t and would
have >2GB values truncated.  Code was added to the SQLite routines to
mitigate those scenarios, essentially potentially resulting in data
truncation.  However you'll notice that various SQLite limits are set to 1GB
or similar so they wouldn't have gone in in the first place.  I believe but
cannot prove that there are potential exploits in this.

> Is it possible to change the fourth parameter in sqlite3_bind_XXX

No.  It would change the size of the parameter which would break the ABI.
You couldn't take something that compiled/linked against current SQLite and
then swap out the shared library for a new one changed like you request.
The only solution would be to add new entry points with different names that
do take (s)size_t.  This could be handled like how the UNIX world introduced
64 bit file sizes and offsets, using the preprocessor to point at the
appropriately sized routines for aware code.

Roger

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Roger Binns
In reply to this post by Richard Hipp-3
On 07/09/14 11:19, Richard Hipp wrote:
> Please use a cast to silence the compiler warnings.  "(int)sizeof(...)"
> instead of just "sizeof(...)".

That isn't safe for correctly written 64 bit apps.  For example they could
end up with data items that are bigger than 2GB correctly using (s)size_t.
The silencing cast above would then truncate it to a negative 32 bit number
or truncated 31 bit number, which has differing meanings in the SQLite 3
APIs, and certainly never matches the callers intention.

The only safe thing for a correctly written 64 bit app to do is ensure that
all size values are less than 2GB, and then the warning can be silenced in a
cast.

Roger

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Simon Slavin-3
In reply to this post by Richard Hipp-3

On 7 Sep 2014, at 7:19pm, Richard Hipp <[hidden email]> wrote:

> [hidden email]> wrote:
>
>> Is it possible to change the fourth parameter in sqlite3_bind_XXX (and
>> probably other locations) because this seems to be for me the appropriate
>> type?!
>
> No.  That would be a compatibility break.

To supplement Richard's answer, I understand that this problem will be solved in SQLite4, which can rely on size_t from the outset.

Simon.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

mailing lists

Am 07.09.2014 um 21:52 schrieb Simon Slavin <[hidden email]>:

>
> On 7 Sep 2014, at 7:19pm, Richard Hipp <[hidden email]> wrote:
>
>> [hidden email]> wrote:
>>
>>> Is it possible to change the fourth parameter in sqlite3_bind_XXX (and
>>> probably other locations) because this seems to be for me the appropriate
>>> type?!
>>
>> No.  That would be a compatibility break.
>
> To supplement Richard's answer, I understand that this problem will be solved in SQLite4, which can rely on size_t from the outset.
>
> Simon.
> _______________________________________________
> sqlite-users mailing list
> [hidden email]
> http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users

Can it also be solved in SQLite3 by overloading versions that support size_t as a parameter?

As Roger pointed out ignoring the warning or implementing just a cast is insufficient. In case of an overloaded function the SQLite limits could also be handled correctly.

Regards,
Hartwig

_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users
Reply | Threaded
Open this post in threaded view
|

Re: Request to change int parameter to size_t parameter / potential bug on iOS (64 bit)

Nelson, Erik - 2
In reply to this post by Roger Binns
> Roger Binns wrote on Sunday, September 07, 2014 2:30 PM
> On 07/09/14 11:19, Richard Hipp wrote:
> > Please use a cast to silence the compiler warnings.
> "(int)sizeof(...)"
> > instead of just "sizeof(...)".
>
> That isn't safe for correctly written 64 bit apps.  For example they
> could end up with data items that are bigger than 2GB correctly using
> (s)size_t.
> The silencing cast above would then truncate it to a negative 32 bit
> number or truncated 31 bit number, which has differing meanings in the
> SQLite 3 APIs, and certainly never matches the callers intention.
>
> The only safe thing for a correctly written 64 bit app to do is ensure
> that all size values are less than 2GB, and then the warning can be
> silenced in a cast.
>

If the Boost C++ libraries are available, there's a numeric_cast that does casting and checks for overflow in the process.

It's not worth adding all of Boost for just this little bit, but if you have it around, may be worth taking a look.

----------------------------------------------------------------------
This message, and any attachments, is for the intended recipient(s) only, may contain information that is privileged, confidential and/or proprietary and subject to important terms and conditions available at http://www.bankofamerica.com/emaildisclaimer.   If you are not the intended recipient, please delete this message.
_______________________________________________
sqlite-users mailing list
[hidden email]
http://sqlite.org:8080/cgi-bin/mailman/listinfo/sqlite-users