-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update to SQLite 3.45.0 introduces an issue where a required column is not passed into a table due to optimizations for the IN keyword #8291
Comments
@Samuel-Roach Thanks for the writing and for catching this. In the sqlite commit it seems this is an optimization done for a |
@Smjert My understanding is that SQLite is not at fault, as the optimisation they have made is probably fair, albeit could be further scrutinised for validation of this. In my opinion, something should be done on the osquery side. I'm not 100% on the interraction between SQLite and Osquery code; however, the concept of required columns for a table is part of osquery, and is not a core part of SQL. Therefore, osquery should make a change in order to accommodate for this optimization made in SQLite. I'm under the impression from the release notes (https://www.sqlite.org/releaselog/3_43_0.html) that this could affect all types of JOIN, as the optimization this is in relation to is generalised to RIGHT and FULL joins. To verify this I have run the following changes to c (
path,
subject_name
) AS (
SELECT
b.path,
ac.subject_name
FROM b
FULL JOIN authenticode AS ac ON
ac.path = b.path
) and c (
path,
subject_name
) AS (
SELECT
b.path,
ac.subject_name
FROM b
RIGHT JOIN authenticode AS ac ON
ac.path = b.path
) In both cases, the end result is as follows, although it takes a little longer to get there:
The query in the initial bug was the minimal reproducible example I could create, and it seems very temperamental when changing any aspect of the query. E.g. Removing the |
I wonder how #8263 intersects this. |
I think this issue might be present in earlier versions, but in a different form. I was experimenting further with using WITH a (
path
) AS (
SELECT
path
FROM services
),
b (
path,
subject_name
) AS (
SELECT
a.path,
authenticode.subject_name
FROM a
INNER JOIN authenticode ON
authenticode.path = a.path
)
SELECT DISTINCT
a.path,
b.subject_name
FROM a
LEFT JOIN b ON
a.path = b.path
WHERE
b.subject_name != 'Microsoft Windows'; Running this against Osquery 5.11.0 I see the following output:
It would be worth noting that changing this to be WHERE
b.subject_name NOT IN ('Microsoft Windows'); also fails, however making the JOIN to authenticode in table |
During the office hours we said that we wanted to resolve this, or at least avoid the regression, by first trying to patch the sqlite source code, removing that optimization. Furthermore there's something that doesn't checks with me. A RIGHT JOIN or a FULL JOIN requires for all the right table rows to be present in the result of the join. The only way one can obtain all results from I can reproduce with your very last example though, which is interesting. But I wonder if there isn't something else we are missing. |
Something I just discovered is that it's possible to return I tried it and it works; I think though the problem with this is when we are actually doing a query with no required constraint. I haven't investigated further but we should also be able to distinguish the case of a plan not be valid due to a required constraint missing among valid plans, to avoid outputting the content of EDIT: Also for additional notes on this, since we talked about this specific thing during the office hours, when the |
See osquery#8291 This also moves the logic to build in osquery, so that's future proof if we ever want to patch sqlite. CVE-2023-7104 is fully resolved by not compiling in the session extension, which was anyway unused.
@Smjert great find! Looks like there is some attempt made to use this functionality but at a glance I can't tell if it's correct: osquery/osquery/sql/virtual_table.cpp Line 992 in 8ded5d5
|
Yeah the difference it's that it's in the |
Ah yes I had just noticed that. It makes sense and seems like a path forward! |
Bug report
What operating system and version are you using?
What version of osquery are you using?
What steps did you take to reproduce the issue?
Running the following query:
What did you expect to see?
Results from the query. This is the seen behaviour in Osquery version 5.11.0.
What did you see instead?
What is the issue?
I have investigated and found the root cause of the issue. It was introduced in SQLite version 3.43.0, Osquery version 5.12.0 will update to SQLite version 3.45.0. I have built Osquery with SQLite version 3.43.0 and seen the issue, and also with SQLite version 3.42.0 and not seen the issue.
An optimisation was introduced as part of the following check-in: https://www.sqlite.org/src/info/f544a8e47cdd5ad7. More specifically, previously
TK_IN
was simply returningWRC_Prune
, however now it will callsqlite3WalkExpr(pWalker, pExpr->pLeft);
beforehand. From my understanding, this is where we see the issue as the IN statement is now walked (and inadvertently now calling theauthenticode
table without the required column).I have built Osquery with SQLite version 3.43.0, and with
TK_IN
back to just callingreturn WRC_Prune;
, and I have seen that the issue is not present, which verifies this. For the moment it can be remediated by changing the last line to the following:However, as this is implying the same as
NOT IN
, they should be interchangeable without error.The text was updated successfully, but these errors were encountered: