github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.69k stars 1.54k forks source link

IRGuardCondition failure to detect NULL condition #15186

Open tardigrade-9 opened 11 months ago

tardigrade-9 commented 11 months ago

I wrote a small query to understand all the IRGuardConditions inside a function

predicate isNotNullCheck3(IRGuardCondition g, Expr e, boolean branch) {

   branch = true
  and 
  (g.getEnclosingFunction().hasName("func2")
  or g.getEnclosingFunction().hasName("xkb_compose_table_new_from_file"))
  and g.getAnOperand().getUse().getUnconvertedResultExpression() = e
}

Below is snippet of my custom function func2(...)

int func2(int num,test_struct *test){
    if(!test) {
        return 1;
    }
    if(num > 0) {
        test = (test_struct *)calloc(num, sizeof(*test));
        if(!test) {
            return 1;
        }
    }
    test[0].a = 1;
    test[0].b = 2;
    return 0;
}

Below is a function named xkb_compose_table_new_from_file from libxkbcommon

xkb_compose_table_new_from_file(struct xkb_context *ctx,
                                FILE *file,
                                const char *locale,
                                enum xkb_compose_format format,
                                enum xkb_compose_compile_flags flags)
{
    struct xkb_compose_table *table;
    bool ok;

    if (flags & ~(XKB_COMPOSE_COMPILE_NO_FLAGS)) {
        log_err_func(ctx, "unrecognized flags: %#x\n", flags);
        return NULL;
    }

    if (format != XKB_COMPOSE_FORMAT_TEXT_V1) {
        log_err_func(ctx, "unsupported compose format: %d\n", format);
        return NULL;
    }

    table = xkb_compose_table_new(ctx, locale, format, flags);
    if (!table)
        return NULL;

    ok = parse_file(table, file, "(unknown file)");
    if (!ok) {
        xkb_compose_table_unref(table);
        return NULL;
    }

    return table;
}

The results for func2 are as expected. I see CompareNE and CompareGT

Screenshot 2023-12-21 at 2 41 44 AM

While for libxkbcommon, I see Load: table and Load: ok as IRGuardCondition rather than CompareNE: (bool)...

Screenshot 2023-12-21 at 2 42 55 AM

Any reason for this discrepancy? Due to this issue, the predicate below is unable to detect NULL check

predicate isNotNullCheck2(IRGuardCondition g, Expr e, boolean branch) {
  g.comparesEq(any(Instruction instr | instr.getUnconvertedResultExpression() = e).getAUse(),
    any(ConstantValueInstruction const | const.getValue() = "0").getAUse(), 0, false, branch)
}
jketema commented 10 months ago

Hi @tardigrade-9,

Which version of CodeQL are you running? This looks like something we fixed recently.

tardigrade-9 commented 10 months ago

Hi @jketema , I'm using CodeQL CLI v2.15.4

jketema commented 10 months ago

Do you work with a cloned copy of the CodeQL repository? If so, what commit are you on?

tardigrade-9 commented 10 months ago

I don't work from cloned copy, I installed using brew and I also use VS Code extension. I tried on v2.15.5 now, the results are still same. Do you want me try from source code build? I also see that 2.15.5 is the latest version.

jketema commented 10 months ago

That shouldn't make a difference. I'll investigate.

jketema commented 10 months ago

Are you sure all your library packages are up-to-date. I'm seeing the following: Screenshot 2023-12-21 at 21 54 38

aeisenberg commented 10 months ago

From the looks of this, you might be using an older version of the codeql/cpp-all library pack. The most recent version is v0.12.2. By default quick queries will just use the latest version of the library pack that is already downloaded. So, if you only have v0.11.2 downloaded, it will use that version.

Can you check in your ~/.codeql/packages/codeql/cpp-all folder and see what is there?

You can force downloading the latest version by running codeql pack download codeql/cpp-all.

tardigrade-9 commented 10 months ago

Thanks @aeisenberg for the suggestion. I did the upgrade to "0.12.2", but the result is still the same. I also tried removing the compile-cache folder, but it did not work too..

tg@TG-MacBook-Air vscode-codeql-starter % cat codeql-custom-queries-cpp/testGuard.ql
import cpp
import semmle.code.cpp.dataflow.new.DataFlow
import semmle.code.cpp.controlflow.IRGuards
predicate isNotNullCheck3(IRGuardCondition g, Expr e, boolean branch) {

    branch = true
   and g.getAnOperand().getUse().getUnconvertedResultExpression() = e
   and g.getEnclosingFunction().hasName("xkb_compose_table_new_from_file")
 }

 from IRGuardCondition g, Expr e, boolean branch
 where isNotNullCheck3(g, e, branch)
    select g, e, branch
tg@TG-MacBook-Air vscode-codeql-starter %
tg@TG-MacBook-Air vscode-codeql-starter %
tg@TG-MacBook-Air vscode-codeql-starter % codeql query run --database ../codeql-db/libxkbcommon/ codeql-custom-queries-cpp/testGuard.ql 
Compiling query plan for /Users/tg/Codebase/vscode-codeql-starter/codeql-custom-queries-cpp/testGuard.ql.
[1/1] Compiled /Users/tg/Codebase/vscode-codeql-starter/codeql-custom-queries-cpp/testGuard.ql.
testGuard.ql: Evaluation completed (804ms).
|           g           |     e      | branch |
+-----------------------+------------+--------+
| BitAnd: ... & ...     | ... & ...  | true   |
| CompareNE: ... != ... | ... != ... | true   |
| Load: table           | table      | true   |
| Load: ok              | ok         | true   |
Shutting down query evaluator.
tg@TG-MacBook-Air vscode-codeql-starter %
tg@TG-MacBook-Air vscode-codeql-starter %
tg@TG-MacBook-Air vscode-codeql-starter % ls ~/.codeql/packages/codeql/cpp-all 
0.12.2
tardigrade-9 commented 10 months ago

UPDATE: I think my vscode-codeql-starter workspace has issue. I tried running the query from a fresh directory , and it worked. Any help to fix the issue with current workspace would be great. Thanks

aeisenberg commented 10 months ago

Ah...yes. That is correct. The starter workspace clones all of the queries in a submodule. I forgot to ask about that. To get the latest queries, just go to the ql submodule in the workspace and git checkout main && git pull.

tardigrade-9 commented 10 months ago

Thanks @aeisenberg for the help.. I can see the results in vscode after submodule update. @MathiasVP provided a guard condition to check NULL condition

predicate isNotNullCheck2(IRGuardCondition g, Expr e, boolean branch) {
  g.comparesEq(any(Instruction instr | instr.getUnconvertedResultExpression() = e).getAUse(),
    any(ConstantValueInstruction const | const.getValue() = "0").getAUse(), 0, false, branch)
 and g.getEnclosingFunction().hasName("xkb_compose_table_new_from_file"))
}

Since the IRGuardCondition is unary instruction, I feel this must be modified. I don't see any result for the above query. @jketema any hints here would be helpful, thanks

MathiasVP commented 10 months ago

I'm not able to download a C/C++ database for the https://github.com/xkbcommon/libxkbcommon/ project, so I can't test it on the "real thing". However, when I run your isNotNullCheck2 predicate on a database containing this snippet:

struct xkb_compose_table;
struct FILE;

enum xkb_compose_format
{
  XKB_COMPOSE_FORMAT_TEXT_V1 = 1
};

enum xkb_compose_compile_flags
{
  XKB_COMPOSE_COMPILE_NO_FLAGS = 0
};

struct xkb_compose_table *xkb_compose_table_new(struct xkb_context *ctx,
                                                const char *locale,
                                                enum xkb_compose_format format,
                                                enum xkb_compose_compile_flags flags);

bool parse_file(struct xkb_compose_table *table, FILE *file, const char *filename);

void xkb_compose_table_unref(struct xkb_compose_table *table);

void log_err_func(struct xkb_context *ctx, const char *fmt, ...);

struct xkb_compose_table *xkb_compose_table_new_from_file(struct xkb_context *ctx,
                                                          FILE *file,
                                                          const char *locale,
                                                          enum xkb_compose_format format,
                                                          enum xkb_compose_compile_flags flags)
{
  struct xkb_compose_table *table;
  bool ok;

  if (flags & ~(XKB_COMPOSE_COMPILE_NO_FLAGS))
  {
    log_err_func(ctx, "unrecognized flags: %#x\n", flags);
    return nullptr;
  }

  if (format != XKB_COMPOSE_FORMAT_TEXT_V1)
  {
    log_err_func(ctx, "unsupported compose format: %d\n", format);
    return nullptr;
  }

  table = xkb_compose_table_new(ctx, locale, format, flags);
  if (!table)
    return nullptr;

  ok = parse_file(table, file, "(unknown file)");
  if (!ok)
  {
    xkb_compose_table_unref(table);
    return nullptr;
  }

  return table;
}
I get 3 results: g e branch
CompareNE: (bool)... ... & ... true
CompareNE: (bool)... table true
LogicalNot: ! ... table false

where:

So I certainly get results from the query. It sounds like you're not getting any results? I'm not sure why that's the case. I also don't follow what you mean by:

Since the IRGuardCondition is unary instruction, I feel this must be modified.

tardigrade-9 commented 10 months ago

@MathiasVP Can you share the database containing the code snippet please? Also you should get 5 results, ok and !ok is missing. I'm assuming you have not omitted the result.

tardigrade-9 commented 10 months ago

libxkbcommon-latest.tar.gz