Uploaded image for project: 'CFEngine Community'
  1. CFEngine Community
  2. CFE-3235

CFEngine string parser blows up or fails on unexpected dollar signs and parentheses

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Open
    • Priority: (None)
    • Resolution: Unresolved
    • Affects Version/s: 3.12.2
    • Fix Version/s: None
    • Component/s: Parsing
    • Labels:
      None

      Description

      I found this and discussed it in great detail, with many examples, in the comments on CFE-3160. However that issue was subsequently closed; since it wasn't primarily about these examples and they weren't addressed, I'm opening this issue separately.

      It's not really a single issue or bug but rather a whole category of issues coming from the design decision to recursively evaluate strings in pretty much all contexts to resolve any and all variables - and the related design decision to assume that anything that looks like a variable reference, IS one in actual fact.

      The basic faulty assumption is: Any time a dollar sign is followed by a parenthesis, it is an attempt to refer to a CFEngine variable.

      (And a related assumption: The above assumption will never cause any security issues.)

      Examples of bugs in this category:

      • The escape() function will be SKIPPED if passed a dollar sign paren sequence - even if the dollar sign is passed explicitly with $(const.dollar). Therefore there is no way to escape a string that contains something that looks to CFEngine like a variable reference.
      • Since regcmp() and strcmp() also SKIP if any argument has something that looks like a variable reference, there is no way to check if a string contains a literal sequence that looks like $(...).

      Note that these first two example bugs affect most if not all standard library edit_line bundles. For a trivial example, it is likely impossible to use a standard library bundle to ensure that a shell script contains the line /bin/echo $(date) because such a line can't be checked for with regline() and can't be passed to escape() either.

      It also extends to filenames:

      • You can create a file whose name has a dollar paren sequence, but you can't find out if it exists (since the fileexists() call will be skipped).

      And it gets worse:

      • You cannot create a variable whose contents are $( (with no close parenthesis). CFEngine will blow up hard.
      • Any attempt to read any string into CFEngine whose contents include a dollar-open-paren sequence (with no close paren) will similarly cause CFEngine to blow up.
      • Creating a file with a $( sequence in its name (and no close parenthesis) will blow up CFEngine if any lsdir() or findfiles() call looks in the directory where the file is placed.
      • Files whose names are valid CFEngine variable references cannot be deleted by CFEngine except by hardcoding $(const.dollar) directly in a files promise promiser.

      This last one means that a CFEngine policy that a particular directory should be empty or should only have certain explicitly named files, cannot be relied upon. (Example: Run touch '/tmp/$(sys.cf_version)' '/tmp/$(sys.ipv4)' and attempt to write a CFEngine policy that will delete those files. Or better yet, run touch '/etc/sudoers.d/$(my_back_door'.)

      There are still other examples of unexpected behavior in my comments on CFE-3160, as well as code illustrating several of the above points.

      One long comment in particular which is worth linking to is https://tracker.mender.io/browse/CFE-3160?focusedCommentId=102118&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-102118 - that comment contains a long but simple CFEngine policy whose results I doubt anyone including the CFEngine developers can correctly predict without actually running it. My offer of a box of cookies still stands.

      All of the above issues stem from the faulty assumption given earlier. This is a language design flaw.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              mweilgart Mike Weilgart
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Summary Panel