On Tue, May 24, 2022 at 12:24 PM Steve Grubb <sgr...@redhat.com> wrote:
> On Tuesday, May 24, 2022 1:55:17 PM EDT Kurt Seifried wrote: > > On Tue, May 24, 2022 at 9:25 AM Steve Grubb <sgr...@redhat.com> wrote: > > > Hello, > > > > > > I am continuing to look at ShellCheck and how to map it's warnings to > > > CWE's. > > > I'm looking at SC2043 - This loop will only ever run once for a > constant > > > value. > > > > > > https://www.shellcheck.net/wiki/SC2043 > > > > > > An example might be: > > > > > > dir=$(ls $HOME) > > > for i in dir > > > do > > > > > > echo $i > > > > > > done > > > > > > which outputs "dir" because it's missing the "$" in the for statement. > > > > > > One of my thoughts is this could be CWE-606: Unchecked Input for Loop > > > Condition. It talks about unchecked inputs causing excessive looping. > > > What > > > about wrong input for loop conditional causing no iteration? > > > > > > Another thought is this could be CWE-670: Always-Incorrect Control Flow > > > Implementation. (But looking at that, I would have expected other bad > > > loop > > > nodes such as CWE-835: Loop with Unreachable Exit Condition.) > > > > > > Is there a better fit? Shell scripting problems really are a hard to > > > match > > > to > > > a CWE because it's problems are similar but very different than C. > > > > Hmm. What about the case where the dev puts in the values, e.g.: > > > > for VARIABLE in file1 file2 file3 > > do > > command1 on $VARIABLE > > done > > This ^^^ works fine. It iterates across the values. > > > which of course could be applied for a single thing as well (to afford > > future flexibility/etc): > > > > for VARIABLE in file1 > > do > > command1 on $VARIABLE > > done > > This ^^^ is another instance of the original issue because there is no > iteration. Could this be CWE-1164? Because you could reduce the whole > thing > down to > There is one iteration. I'm not going to argue semantics, but I would point out I've seen code with loops of one because of future growth, or because various options were removed and it's easier than refactoring the code. > > command file1 > > By removing deadcode. And that is what the warning is about. It's to say > take > a look at this - it doesn't make sense to go through the trouble of making > it > look like you wanted to loop and then you don't. > If we make removing deadcode a requirement... I'm going to be so glad I got out of the security response in OpenSource business =) > > > and then the question is "did the programmer mean to have a variable > called > > "dir" and an actual instance of a file or directory or whatever called > > "dir"? Probably not but maybe yes? > > Or they meant to add a /* after file1? In any event, I don't really find > any > satisfactory matching CWE. > > > > Maybe a more generic along the lines of are you using variables and > values > > that happen to share the same naming, e.g. "$dir" and "dir" which makes a > > mess much easier? > > And this is how the domain of shell differs from C or Java. Shell is happy > to > use the constant string "dir" instead of the variable $dir. It likely > isn't > what was intended. Or at least it deserves a look. > Maybe a "dangerous use of same string to identify different things" or "lack of variable prefix"? > > -Steve > > > -- Kurt Seifried (He/Him) k...@seifried.org
