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.

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

bug: Improve chassis_types and security_breach columns within chassis_info #6608

Merged
merged 2 commits into from
Aug 26, 2020

Conversation

farfella
Copy link
Contributor

Fix for bug #6538

  • Correctly returns chassis type if only one chassis type.
  • Correctly returns multiple chassis types.
  • Table schema has been updated so that this column is now text and not an integer.

@@ -205,6 +205,16 @@ class WmiResultItem {
Status GetVectorOfStrings(const std::string& name,
std::vector<std::string>& ret) const;

/**
* @brief Windows WMI Helper function to retrieve a vector of long result
* from
Copy link
Member

Choose a reason for hiding this comment

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

nitpick, formatting, no need to have a newline here

std::ostringstream oValueConcat;
for (size_t i = 0; i < chassisTypes.size(); ++i) {
if (i != 0) {
oValueConcat << ",";
Copy link
Member

Choose a reason for hiding this comment

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

nitpick, do you want this to read 1,2,3,4 or 1, 2, 3, 4? If you do not want the space maybe you can << ','.

@@ -3,7 +3,7 @@ description("Display information pertaining to the chassis and its security stat
schema([
Column("audible_alarm", TEXT, "If TRUE, the frame is equipped with an audible alarm."),
Column("breach_description", TEXT, "If provided, gives a more detailed description of a detected security breach."),
Column("chassis_types", INTEGER, "The type of chassis, such as Desktop or Laptop."),
Column("chassis_types", TEXT, "The type of chassis, such as Desktop or Laptop."),
Copy link
Member

Choose a reason for hiding this comment

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

A comma-separated list of chassis types, such as Desktop or Laptop.

@farfella
Copy link
Contributor Author

Thanks for the review @theopolis ! I updated the data in the column so chassis_types now has readable values, e.g., Laptop, vs integral values.

Also, noticed that security_status column was actually presenting values from SecurityBreach field (and not the separate SecurityStatus field) so I updated this to security_breach now. This is an unsigned short field instead of a long (ref: https://docs.microsoft.com/en-us/windows/win32/cimwin32prov/win32-systemenclosure), and it may in fact be non-existent. So, there's now a check to return Unknown (2) if it is non-existent instead of returning garbage as what was happening before.

@theopolis theopolis changed the title bug-6538: return chassis_types array as string bug: Improve chassis_types and security_breach columns within chassis_info Aug 26, 2020
@theopolis theopolis merged commit a19d910 into osquery:master Aug 26, 2020
@theopolis
Copy link
Member

This is great!

Heads up that I normally recommend a "one thesis rule", meaning each PR should change one distinct thing. I renamed this to indicate that you are essentially improving the data within both of these columns.

@farfella
Copy link
Contributor Author

I agree with the one thesis rule. :) Integration test was failing because result of security_breach was uninitialized sometimes.

@farfella farfella deleted the bug-6538 branch August 26, 2020 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants