soci::use allows taking the address of rvalues which can easily lead to stack-use-after-scope UB

[StefanVK]

sql << insert, soci::use(std::string("this is a bug and for a long string may actually segfault")); // stack use after scope sql << insert, soci::use(7); // also stack use after scope - unlikely to fail in real life except in address sanitizer builds

In the soci unittests there once was a bug https://github.com/SOCI/soci/issues/657 which was fixed by https://github.com/SOCI/soci/pull/659/ . The problem is that if you soci::use an temporary, it will get deleted at the end of the statement. The sql statement will also be evaluated at the end of the expression, triggered by the destructor of once_temp_type. The temporary may be deleted first though and then in the statement execution we access the already invalid temporary.

Minimal example code that shows the core of the problem with address sanitizer: https://godbolt.org/z/76nPdToYx

I propose deleting soci::use for rvalues so this type of erroneous code no longer compiles:

template <typename T>
details::use_container<T, details::no_indicator> use(T && t, const std::string &name = std::string()) = delete;

template <typename T>
details::use_container<const T, indicator> use(T && t, indicator & ind, std::string const &name = std::string()) = delete;

For the bug fixed by https://github.com/SOCI/soci/pull/659/ we would have gotten an error message such as:

[build] D:\proj\soci\tests\common-tests.h(2309): error C2280: 'soci::details::use_container<const tm,soci::indicator> soci::use<tm>(T &&,soci::indicator &,const std::string &)': attempting to reference a deleted function
[build]         with
[build]         [
[build]             T=tm
[build]         ]
[build] D:\proj\soci\include\soci\use.h(53): note: see declaration of 'soci::use'

Instead of code that compiles but has UB.

Do you think it could break valid code?

[vadz]

Thank you Stefan, this looks like an excellent idea and I'm pretty sure it hadn't been done originally just because SOCI was written in C++98 which didn't provide any way to distinguish between l- and r-value references. If/when the PR CI builds pass, I'm going to merge it.

[vadz]

It is a bit annoying that we can't do soci::use(7) any more, it looks like it ought to be possible to make this to work, but this would probably require using values instead of pointers for primitive types which doesn't seem simple... Would you have any idea about how could we still allow it by chance?

If not, the PR needs to be updated to change the Oracle unit tests to avoid using this.

[StefanVK]

Thank you, @vadz!

I had missed the use in the oracle test since I had only built it locally without having oracle, sorry. I have now updated the unit test to avoid the rvalue there as well. But I totally agree that having soci::use work for rvalues of primitive types would be quite desirable and it should be possible to instead save those by value like you suggested. I will take a look, whether I can come up with an implementation proposal for that.