SQLite User Forum

New security rules(NULL check)
Login

New security rules(NULL check)

(1) By icy17 (amberliu) on 2024-07-20 16:55:49 [source]

Hi, I've observed that some security rules are absent from the documentation.

For the security rule "parameters must not be NULL," its omission in the documentation can lead to significant security issues.

Firstly, while best programming practices dictate that developers should avoid passing NULL parameters to an API, this guideline may not be obvious to less experienced developers, increasing the risk of overlooking necessary security checks.

Secondly, many APIs receive parameters from other functions or APIs, which might not be adequately checked for NULL, posing a risk even for experienced developers.

Therefore, it is crucial to explicitly mention in the documentation that NULL checks must be conducted on specified parameters before an API is used. Including these security rules will help developers identify potential security issues more effectively during testing.

I will provide a list of APIs requiring checks along with their corresponding parameters:

API parameter index (begin with 1)
sqlite3_prepare_v2 4
sqlite3_str_append 1
sqlite3_value_bytes 1
sqlite3_snprintf 2
sqlite3_snprintf 3
sqlite3_result_int 1
sqlite3_value_int 1
sqlite3_result_null 1
sqlite3_value_type 1
sqlite3_value_int64 1
sqlite3_result_double 1
sqlite3_result_error 1
sqlite3_user_data 1
sqlite3_file_control 1
sqlite3_file_control 2
sqlite3_str_reset 1
sqlite3_value_double 1
sqlite3_declare_vtab 1
sqlite3_value_numeric_type 1
sqlite3_db_config 1
sqlite3_create_module 1
sqlite3_column_name 1
sqlite3_db_status 1
sqlite3_last_insert_rowid 1
sqlite3_str_appendchar 1
sqlite3_vtab_config 1
sqlite3_changes 1
sqlite3_open_v2 2
sqlite3_open 2
sqlite3_result_subtype 1
sqlite3_clear_bindings 1

(2) By Stephan Beal (stephan) on 2024-07-20 17:16:08 in reply to 1 [link] [source]

... can lead to significant security issues.

Presumably the timing of this and the Crowdstrike meltdown, which was reportedly caused by a NULL pointer dereference, is no coincidence?

If you have concrete security issues to report, and do not wish to publicize them, please send them to our project lead: drh at this domain. Generic cries of "security issue" are unlikely to gain any significant traction here without demonstration of their real-world applicability.

(3) By Richard Hipp (drh) on 2024-07-20 17:47:10 in reply to 1 [link] [source]

I came up in a time when "parameters must not be NULL" was the default assumption, and you were only allows to pass in NULL to pointer parameters if the documentation specifically allowed it. Maybe its just me, but it seems like developers who don't know that ought not be programming security-sensitive applications in C/C++ in the first place?

Prove me wrong, but my guess is that adding lots of verbiage about parameters-must-not-be-NULL to the documentation will just clutter things, and obscure the important stuff.

(4) By anonymous on 2024-07-21 05:26:47 in reply to 3 [link] [source]

I get the impression that sort of documentation (spelling out what you call 'default assumptions') is really to help drive AI programming and automated coding tools.

If these requests are really about helping AI/automated tools, then it seems to me that the AI/automated tools might want to work on creating their own 'thou shall not do silly things' rules rather than forcing humans to start reading documentation written for AI.

(5) By Warren Young (wyoung) on 2024-07-21 06:12:47 in reply to 1 [link] [source]

best programming practices dictate that developers should avoid passing NULL parameters to an API

That's the problem with "best practices:" who defines "best," and by what criteria? When the who and the what change, "best" often changes, meaning it was not objectively "the best" in the first place.

Just in the last week, I recall passing NULL to at least two different C APIs on purpose in order to get intended behavior.

(6) By Spindrift (spindrift) on 2024-07-21 06:33:39 in reply to 5 [link] [source]

And I suppose the question pertinent to the topic of this thread is whether the documentation for those APIs explicitly described the role and acceptability of feeding them with NULL, or was it just implicitly assumed that they would behave correctly?

(7) By Nuno Cruces (ncruces) on 2024-07-21 11:48:17 in reply to 5 [link] [source]

I assume DRH's point is: if it's not stated otherwise, you assume an API only accepts correctly initialized pointers.

APIs that are supposed to work with NULL pointers, explicitly state so in the docs.

Do we really need every API to state that sqlite3* needs be a valid connection? Isn't it more sensible to just say sqlite3_close is a NOP on NULL?

(8) By icy17 (amberliu) on 2024-07-22 09:39:51 in reply to 7 [link] [source]

Perhaps a better way would be to add the security notes to APIs that may `create' NULL pointers, such as: "Make sure to check the return value." I've found that a lot of API misuse is due to developers forgetting or not knowing to check for NULL return values, which leads to problems in subsequent use.

(9) By Nuno Cruces (ncruces) on 2024-07-22 11:12:39 in reply to 8 [link] [source]

Most SQLite APIs return an error code, not a pointer.

And most that do return an error code, but have a pointer argument for a second return value, do state explicitly what they do with it in case of error (set it to NULL, set it to something else that needs to be freed, don't touch it if it's NULL thus optional, etc).

If there are specific examples to the contrary, I guess doc improvements would always be great. But blanket statements, or worse specific comments that just turn out to be wrong on close inspection, just create fear mongering and bring nothing useful.

All I got from both these threads was wasted time on deep dives to check assumptions that all turned out as being as-expected/as-specified (according to docs) not as-described (according to forum posts).

(10) By Stephan Beal (stephan) on 2024-07-22 12:04:14 in reply to 9 [link] [source]

All I got from both these threads was wasted time on deep dives to check assumptions that all turned out as being as-expected/as-specified (according to docs) not as-described (according to forum posts).

FWIW, it wasn't entirely wasted: i had committed to looking over the non-NULL-related submissions when i was back on the computer, and your work saved me from having to do that ;). Thank you for that :).

Specifically regarding the NULLs, though: SQLite has a build-time config option called SQLITE_API_ARMOR which enables "superfluous" checks for arguments which must not be NULL or which might refer to out-of-bounds array access and the like. When that option is enabled, some APIs which have "undefined behavior" (in the C sense of the term) change to have "undefined behavior" in a more limited sense: they may, e.g., return result codes which the API docs do not mention (e.g. returning SQLITE_MISUSE where that code is otherwise not documented). They won't (or shouldn't), however, dereference a NULL. One problem case here is functions which return void: in SQLITE_API_ARMOR mode, such functions will simply return without doing anything useful and that may lead to difficult-to-find downstream errors.

SQLITE_API_ARMOR was initially added for use on mobile OS platforms, in order to protect such apps against outright dying due to, e.g., a null-pointer dereference. It is is also used by default in the JNI and WASM builds. It is not, however, used in any of this project's "native" builds.

(11) By Nuno Cruces (ncruces) on 2024-07-22 12:54:03 in reply to 10 [link] [source]

FWIW, it wasn't entirely wasted: i had committed to looking over the non-NULL-related submissions when i was back on the computer, and your work saved me from having to do that ;). Thankou for that :).

Great! Though I can't promise I was exhaustive.

I went through the ones that seemed unexpected (specifically one that would make my usage seriously broken, and would probably affect a bunch of other APIs) and checked those out.

(12) By icy17 (amberliu) on 2024-07-22 15:11:39 in reply to 11 [link] [source]

Thank you for your response. I sincerely apologize for wasting your time with suggestions that may not have made sense due to my limited understanding of the specific scenarios in which many APIs are used.

(13) By jose isaias cabrera (jicman) on 2024-07-22 20:32:26 in reply to 12 [link] [source]

These questions are kept as responses/answers to others that may have the same questions, or like it. It's not a waste of time. If they are sincere, they are always useful.