Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update some sqlite types #6392

Merged
merged 2 commits into from
Apr 28, 2020
Merged

Update some sqlite types #6392

merged 2 commits into from
Apr 28, 2020

Conversation

directionless
Copy link
Member

@directionless directionless commented Apr 15, 2020 •
edited

sqlite defines SQLITE_INTEGER as 64bit, not 32bit. Most of our usage seems correct, but a couple did not. This updates them

Also include SQLITE_BLOB in ATC tables.

Fixes: #6391

Not sure how to add tests for this.

dover:build seph$ ./osquery/osqueryi  --config_path  /tmp/osq.conf "select * from url_timestamp limit 4;"
W0414 23:25:26.263969 284452288 auto_constructed_tables.cpp:178] ATC Table: url_timestamp is misconfigured. The configuration include `path`, which is a reserved column
+-------------------------------------------------------------------------+-------------------+---------------------+
| path                                                                    | time              | time_converted      |
+-------------------------------------------------------------------------+-------------------+---------------------+
| /Users/seph/Library/Application Support/Google/Chrome/Profile 1/History | 13231386168665043 | 2020-04-15 01:02:48 |
| /Users/seph/Library/Application Support/Google/Chrome/Profile 1/History | 13231386168665043 | 2020-04-15 01:02:48 |
| /Users/seph/Library/Application Support/Google/Chrome/Profile 1/History | 13227724671117522 | 2020-03-03 15:57:51 |
| /Users/seph/Library/Application Support/Google/Chrome/Profile 1/History | 13227724671117522 | 2020-03-03 15:57:51 |
+-------------------------------------------------------------------------+-------------------+---------------------+

@directionless
Update some sqlite types
ecd5a79 
sqlite [defines](https://www.sqlite.org/c3ref/column_blob.html) `SQLITE_INTEGER` as 64bit, not 32bit. Most of our usage seems correct, but a couple did not. This updates them

Also include `SQLITE_BLOB` in ATC tables.

Fixes: kolide/k2#2054
@Smjert Smjert requested review from Smjert and removed request for Smjert April 15, 2020 11:09
Smjert
Smjert approved these changes Apr 15, 2020
View reviewed changes
Copy link
Member

@Smjert Smjert left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for debugging and fixing this!

If you want to do a regression test, one thing that it comes to my mind is to just reproduce what you basically did to find the issue in the first place.
Meaning, from code you could create a very simple sqlite database with an integer value that is bigger than a 32bit int (or any other value type you want to test), then provide a config for the ATC table to

osquery/plugins/config/parsers/auto_constructed_tables.cpp

Line 109 in ae66d8f

Status ATCConfigParserPlugin::update(const std::string& source,

then query the created table and verify that the value retrieved matches.

You could also pre-generate the config and sqlite db, but especially for the sqlite db it would be a bit less flexible and a bit opaque.

@directionless
Copy link
Member Author

I'm not going to manage tests for this. Merging as is

@directionless directionless merged commit f54d904 into osquery:master Apr 28, 2020
@directionless directionless deleted the seph/atc-64-bit branch April 28, 2020 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Smjert Smjert Smjert approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

32bit/64bit type mismatch in ARC tables
2 participants
@directionless @Smjert