Bug categorization #181
Description
Basically, the two bullet points in each item follow the two questions @tylergu mentioned in Slack:
We need to think about how does the design of the operators lead to the bugs. Does the runtime or programming model make it easier to write these bugs? Or the developers are just careless?
What makes Acto to be able to find these bugs? Can we exploit some of the patterns to find more bugs?
Wrong logic for non-multiple-choice problems
Silently cast to default if invalid: (pingcap/tidb-operator#4650)
- If there are two valid options A and B, developers will assume B is chosen if A is not. This is not a multiple-choice problem since developers have not limited the available input, while in the operator code, they assume that the input will be within a certain range of items.
- Acto can find it because the input it generates is likely to be invalid if developers do not limit the available input in
types.gousingkubebuilder.
Wrong way of checking
Empty value: (pingcap/tidb-operator#4635)
- Developers only check the length of a certain field (which is an array) to validate whether the input has all required fields specified. And if the length ≥ 3, they will consider it as valid input. However, it is possible that the input contains 3 empty items, which essentially mean nothing, and this array of empty items will bypass the validation.
- Acto can find it because Acto generates the most basic value that can satisfy the requirement. Since the operator does not prohibit empty items in an array, Acto just does that.
Carelessness
Invalid image name: (cockroachdb/cockroach-operator#922)
- We only have one case for this in crdb-operator. The developers assume that there is always a colon in the image name specified by the user. If there is no colon, the program will crash.
- Acto can detect it because a colon is not required explicitly so the input it generates does not have a colon, which is invalid.
Required fields are not required explicitly: (pingcap/tidb-operator#4633)
-
More than 100 true alarms are caused by this. I think this is just because of the carelessness of developers.
Think of this: if developers do either of the following work (which they should have done both), this bug will not show up:
- explicitly require the presence of these required fields in
types.go - add validation to check whether required fields are present or not
Neither these two steps are done. So the input is rejected from the server side.
- explicitly require the presence of these required fields in
-
Acto can find it because the generated input does not have these required fields, which is invalid. In turn, pods will never be up.
Quantity conversion: (kubernetes/kubernetes#110654)
- We found 3 bugs (all fixed now) in it. Most likely that developers are careless and do not take some extreme cases into consideration.
- Acto generates many “nonsense” values (e.g. negative number, extremely large number, extremely long number) as input, which trigger bugs.
Find more bugs
- From my perspective, quantity conversion is error-prone. We may find more bugs in it if we deliberately inject a lot of input to all fields which contain quantities. But I am not sure whether doing so conforms to Acto’s intention, as it sounds more like a fuzzing technique.
- "if not A then B" seems to be a pattern which can be further explored: there are more than 1 place where TiDB developers apply this logic.
- Another possible pattern is that when checking whether certain sub-fields are defined in a field of type array, developers only check the length of that array, without validating its content.