Skip to content

plist: fix inverted strcmp in string to boolean conversion - #317

Open
arpitjain099 wants to merge 1 commit into
libimobiledevice:masterfrom
arpitjain099:fix/dict-get-bool-inverted-strcmp
Open

plist: fix inverted strcmp in string to boolean conversion#317
arpitjain099 wants to merge 1 commit into
libimobiledevice:masterfrom
arpitjain099:fix/dict-get-bool-inverted-strcmp

Conversation

@arpitjain099

Copy link
Copy Markdown

plist_dict_get_bool() compares the string value with strcmp() but treats a non-zero return as a match. strcmp() returns 0 on equality, so both conditions are inverted:

if (strcmp(strval, "true")) {
    bval = 1;
} else if (strcmp(strval, "false")) {
    bval = 0;
} else {
    PLIST_ERR("%s: invalid string '%s' for string to boolean conversion\n", __func__, strval);
}

"true" takes the first comparison to 0, falls to the else if, where strcmp("true", "false") is non-zero, and sets bval = 0. "false" matches the first condition and sets bval = 1. Anything else also sets bval = 1. The error branch is unreachable, because reaching it would need both comparisons to return 0 for the same string.

Behaviour of the two forms:

input          before   after
true           0        1
false          1        0
not-a-bool     1        error
TRUE           1        error
(empty)        1        error

So today the API returns the opposite of the stored value for both valid boolean strings, and returns true for strings that are not booleans, rather than reporting the conversion error that the PLIST_ERR call was written for.

The fix compares == 0 in both conditions. plist_dict_copy_bool() goes through the same helper and is fixed by the same change.

Worth knowing for anyone reading this later: applications that use plist_dict_get_bool() on parsed input to read a flag have been getting the inverted answer, so a value of "false" in a plist or JSON document reads as true.

plist_dict_get_bool() compared the string value with strcmp() but treated a
non-zero return as a match. strcmp() returns 0 on equality, so the conditions
were inverted: "true" produced 0, "false" produced 1, and any other string
also produced 1. The error branch could never be reached, since it required
both comparisons to return 0 at once.

The result is that the API returns the opposite of the stored value for both
valid boolean strings, and returns true for strings that are not booleans at
all, instead of reporting the conversion error.

Compare == 0 in both conditions:

  input          before   after
  true           0        1
  false          1        0
  not-a-bool     1        error
  TRUE           1        error
  (empty)        1        error

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant