Extra spaces added by bandit.code.utils.concat_string

[alistairwatts]

Describe the bug

When calling bandit.code.utils.concat_string extra spaces are added between the strings which are concatenated.

Reproduction steps

import ast
from bandit.core import utils

# Create a simple string concatenation
bin_op = ast.parse('"Hello" + "World"').body[0].value

# Manually fix-up the node
bin_op._bandit_parent = None

node, string = utils.concat_string(bin_op)

assert string == "HelloWorld", 'Got %r' % (string, )

Expected behavior

Extra spaces should not be added.

Remove the space used to .join() the strings on

bandit/bandit/core/utils.py

Line 305 in 72fa5a7

return (node, " ".join([x.s for x in bits if isinstance(x, ast.Str)]))

Bandit version

1.7.5 (Default)

Python version

3.9

Additional context

No response

[rkuczer]

Looks like all that is needed is to delete the space in the return statement

[ericwb]

concat_string is an internal function with a specific use. The problem as stated above implies this is a open function for any application. That is not its intention. Please describe how this affects Bandit and ideally an example use case. For example, how does it affect the injection_sql plugin?

[alistairwatts]

As I understand it concat_string is an internal function for revealing a string which has non-trivial construction within the source code. A contrived example for the injection_sql plugin would be something like

sneaky_sql = 'SEL'+'ECT * FROM table1 WHERE %s' % (param, )

This wouldn't be detected. Whilst I'm not really concerned that this is a likely use case, I am looking into analyzing other data contained within strings which could be a security issue. It seemed reasonable to use the concat_string function to deal with non-trival strings, which is how I found this issue.

I'm happy to put up a pull request to fix the issue, but the fix is just to remove the space from the .join

[OClark23]

tested and forked on my branch adding space rewritten func

string_bits = [] for bit in bits: if isinstance(bit, ast.Str): string_bits.append(bit.s) string = " ".join(string_bits) result = (node, string) return result

Explanation: empty list called string_bits, then iterates through the list of bits. For each bit that is an instance of the ast.Str class, its s attribute (which contains the string value) is appended to string_bits. Finally, the string_bits list is joined into a single space-separated string using the join method and assigned to the string variable. The result is then returned as a tuple containing a node and string

[DanOwens02]

@OClark23
What is the purpose of creating an empty list called string_bits and iterating through a list called bits in this code implementation? How does the code identify instances of the ast.Str class and what does it do with them? How is the string_bits list transformed into a space-separated string and what is the resulting tuple that is returned by the code?

[OClark23]

@DanOwens02 The code implementation is to extract string values from an abstract syntax tree and concatenate them into a single space-separated string that can be used for further processing. It would be easier to add a test for the code implementation you provided because it separates the string extraction process into a more readable and manageable format. it is all preference.