fkie-cad / FACT_docker

Dockerfile for building the FACT container
GNU General Public License v3.0
21 stars 10 forks source link

Source Code Analysis Errors #10

Closed frakman1 closed 2 years ago

frakman1 commented 2 years ago

I uploaded a zip of the curl source code and selected the 'source code analysis' checkmark plugin only. When it was done, any .c file I opened displayed "Error during analysis" :

image

From the logs, the only relevant thing I saw was this:

[2021-10-31 02:11:27][docker][WARNING]: [source_code_analysis]: encountered process error while processing [2021-10-31 02:11:28][docker][WARNING]: [source_code_analysis]: encountered process error while processing

maringuu commented 2 years ago

What version of fact are you using?

Are you using start.py? If you aren't this might be the problem here. The plugin tries to start a docker container and mount the firmware files into the container. Since the fw_data_path is inside the FACT_docker container it will not be mounted inside the container started by the plugin (which uses docker on the host).

frakman1 commented 2 years ago

I am using version FACT 3.3-dev I don't think I am using start.py since I ran it using something like:

docker run -it --name fact --net=host --group-add $(getent group docker | cut -d: -f3)  -v /media/data:/media/data -v /var/run/docker.sock:/var/run/docker.sock -v /tmp/fact-docker-tmp:/tmp/fact-docker-tmp -p 0.0.0.0:5000:5000 -p 0.0.0.0:27018:27018/tcp  frakman1/fact:latest start

I think I understand what you mean. The path that the plugin needs (fw_data_path) in the DID (docker-in-docker) is not shared with the parent fact docker container through a -v volume mount option. Is there some workaround to this?

jstucke commented 2 years ago

It seems like an unrelated change to this plugin in https://github.com/fkie-cad/FACT_core/pull/655 also fixes this problem by accident. At least it seems to work for me after I updated my FACT in the container

The only problem is that this needs a new docker container so you may need to rebuild your image. There may still be workarounds to this like cloning the FACT_core repo and running plugins/analysis/linter/install.py to build the container but there could be other problems.

frakman1 commented 2 years ago

OK, it seems I need to start over again. Now that the readme no longer has docker run commands and uses start.py instead, can you help me with a couple of items please? I see that:

--wt-mongodb-path maps to /media/data/fact_wt_mongodb --fw-data-path maps to /media/data/fact_fw_data

I normally map the volume -v /media/data:/media/data in my docker run syntax. What about the folder fact_auth_data which is inside /media/data ?

Also, the readme says:

If you use --config and change anything related to the mongodb database ...

But I don't see --config mentioned anywhere in the readme and start.py --help doesn't say anything about --config.

It also mentions --docker-dir which I also don't see mentioned anywhere. I think the Usage section is very confusing since it mentions things that are not in the repo (main.cfg)

Then there is an Entrypoint section, but that doesn't make sense to me if I am no longer using docker run and am supposed to use start.py now.

jstucke commented 2 years ago

It seems we have still room for improvement regarding the readme.

What about the folder fact_auth_data which is inside /media/data ?

This folder contains the user database which is only relevant if you have authentication enabled (authentication = true in main.cfg)

But I don't see --config mentioned anywhere in the readme and start.py --help doesn't say anything about --config

You can use start.py run --help to get more information and it should actually be --config-path. This parameter is meant for mounting a custom config folder inside the container (a more persistent way than docker exec)

jstucke commented 2 years ago

It also mentions --docker-dir which I also don't see mentioned anywhere. I think the Usage section is very confusing since it mentions things that are not in the repo (main.cfg)

--docker-dir DOCKER_DIR The path on the host that the container should instruct docker to output files generated by running other docker images. (default: /tmp/fact-docker-tmp)

You normally shouldn't have to change this path.

Then there is an Entrypoint section, but that doesn't make sense to me if I am no longer using docker run and am supposed to use start.py now.

This is the argument is passed inside the container and it is not really relevant for you. We will probably remove this section.

jstucke commented 2 years ago

Did you get it to run again? If so, does the plugin work without errors?

frakman1 commented 2 years ago

I have not tried to start over from scratch using the new method. I was really hoping I could make an update within the docker container somehow.

jstucke commented 2 years ago

It should still be possible to update the container. It just gets more complicated: the new version of the plugin "source_code_analysis" switched from jshint to eslint and uses a new docker container for that. You can simply do a git pull and restart the container but I don't know if its possible to simply rerun the updated plugin installation. It should be sufficient to simply pull the container (docker pull cytopia/eslint), though (from outside the container) since nothing else needs to be installed.

jstucke commented 2 years ago

You could also rebuilt the container with a different name/tag (use the --tag parameter of start.py build and the --image parameter of start.py run) to test if the latest build works (you should not loose any data when switching containers or images , since the DB is located outside the container).

frakman1 commented 2 years ago

I performed a docker pull cytopia/eslint and restarted the fact container and forced a re-scan of the .c file but still got the same error shown in the screenshot above. The logs from the WebUI's Backend page show:

[2021-11-09 18:05:57][db_interface_statistic][ERROR]: Could not store statistic backend (localhost:27018: [Errno 111] Connection refused, Timeout: 30s, Topology Description: <TopologyDescription id: 6184e1fde3fe036c68cc857c, topology_type: Single, servers: [<ServerDescription ('localhost', 27018) server_type: Unknown, rtt: None, error=AutoReconnect('localhost:27018: [Errno 111] Connection refused')>]>)
Traceback (most recent call last):
  File "/opt/FACT_core/src/storage/db_interface_statistic.py", line 35, in update_statistic
    self.statistic.delete_many({'_id': identifier})
  File "/usr/local/lib/python3.8/dist-packages/pymongo/collection.py", line 1292, in delete_many
    self._delete_retryable(
  File "/usr/local/lib/python3.8/dist-packages/pymongo/collection.py", line 1206, in _delete_retryable
    return self.__database.client._retryable_write(
  File "/usr/local/lib/python3.8/dist-packages/pymongo/mongo_client.py", line 1551, in _retryable_write
    with self._tmp_session(session) as s:
  File "/usr/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.8/dist-packages/pymongo/mongo_client.py", line 1948, in _tmp_session
    s = self._ensure_session(session)
  File "/usr/local/lib/python3.8/dist-packages/pymongo/mongo_client.py", line 1935, in _ensure_session
    return self.__start_session(True, causal_consistency=False)
  File "/usr/local/lib/python3.8/dist-packages/pymongo/mongo_client.py", line 1883, in __start_session
    server_session = self._get_server_session()
  File "/usr/local/lib/python3.8/dist-packages/pymongo/mongo_client.py", line 1921, in _get_server_session
    return self._topology.get_server_session()
  File "/usr/local/lib/python3.8/dist-packages/pymongo/topology.py", line 520, in get_server_session
    session_timeout = self._check_session_support()
  File "/usr/local/lib/python3.8/dist-packages/pymongo/topology.py", line 499, in _check_session_support
    self._select_servers_loop(
  File "/usr/local/lib/python3.8/dist-packages/pymongo/topology.py", line 218, in _select_servers_loop
    raise ServerSelectionTimeoutError(
pymongo.errors.ServerSelectionTimeoutError: localhost:27018: [Errno 111] Connection refused, Timeout: 30s, Topology Description: <TopologyDescription id: 6184e1fde3fe036c68cc857c, topology_type: Single, servers: [<ServerDescription ('localhost', 27018) server_type: Unknown, rtt: None, error=AutoReconnect('localhost:27018: [Errno 111] Connection refused')>]>
[2021-11-09 18:08:29][docker][WARNING]: [source_code_analysis]: encountered process error while processing
frakman1 commented 2 years ago

You can simply do a git pull and restart the container

I don't understand the relationship between the fact container and the git repo. I used the repo (FACT_docker) to build the fact container then I ran it. What will git pull do? Is there a repo inside the container that I can do this on? Updating the repo outside the container won't make a difference unless I rebuild the docker image and re-run a new container from it.

jstucke commented 2 years ago

Sorry that was a bit ambiguous. What I meant was doing a git pull in the FACT_core folder inside the container to get the latest changes to the plugin.

frakman1 commented 2 years ago

Ahh thank you! That's what I thought originally and searched for a .git folder in the container but I didn't use quotes so I missed it. I did a git pull within the fact container in /opt/FACT_core, restarted the container and now the source code analysis component works:

image

However, I was surprised that it did not support the .c/.cpp file type (my original test) until I looked deeper and see that it really is a script linter with support for only the following:

    SCRIPT_TYPES = {
        'shell': {'mime': 'shell', 'shebang': 'sh', 'ending': '.sh', 'linter': shell_linter.ShellLinter},
        'lua': {'mime': 'luascript', 'shebang': 'lua', 'ending': '.lua', 'linter': lua_linter.LuaLinter},
        'javascript': {'mime': 'javascript', 'shebang': 'javascript', 'ending': '.js', 'linter': js_linter.JavaScriptLinter},
        'python': {'mime': 'python', 'shebang': 'python', 'ending': '.py', 'linter': python_linter.PythonLinter}
    }

I think the name "source code analysis" may be a little misleading to someone who is expecting source code like c/cpp files to be analyzed. I see now that the hover/pop-up text does say that it is for scripts but perhaps better name can be used (e.g. script linter/analyzer)

jstucke commented 2 years ago

I think the name "source code analysis" may be a little misleading to someone who is expecting source code like c/cpp files to be analyzed. I see now that the hover/pop-up text does say that it is for scripts but perhaps better name can be used (e.g. script linter/analyzer)

I agree. It wouldn't be a problem to extend the plugin to other (and also compiled) languages. We usually don't see many of those files, though. For example in my local database there are only 7 C source files and all stem from the same device (in contrast, there are 1734 javascript files). Is there anything concrete that you are missing (e.g. https://github.com/oclint/oclint)? Maybe this is a good opportunity to improve the plugin.

frakman1 commented 2 years ago

I wasn't sure of the use-case of the tool either. I was wondering if I could upload a zip of the source code that was used to build the firmware. That way, the "software components" list might be more complete and therefore the resultant CVE list, more meaningful.

Static analysis results of c/cpp code using a tool like cppcheck would be helpful. However, those results tend to be overwhelmingly large which is where the comparison feature between versions could be useful.

I haven't tried oclint. I will look into it. Thanks