Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IRGuardCondition failure to detect NULL condition #15186

Open
tardigrade-9 opened this issue Dec 21, 2023 · 14 comments
Open

IRGuardCondition failure to detect NULL condition #15186

tardigrade-9 opened this issue Dec 21, 2023 · 14 comments
Labels
C++ question Further information is requested

Comments

@tardigrade-9
Copy link

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)
}
@tardigrade-9 tardigrade-9 added the question Further information is requested label Dec 21, 2023
@aeisenberg aeisenberg added the C++ label Dec 21, 2023
@jketema
Copy link
Contributor

jketema commented Dec 21, 2023

Hi @tardigrade-9,

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

@tardigrade-9
Copy link
Author

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

@jketema
Copy link
Contributor

jketema commented Dec 21, 2023

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

@tardigrade-9
Copy link
Author

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
Copy link
Contributor

jketema commented Dec 21, 2023

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

@jketema
Copy link
Contributor

jketema commented Dec 21, 2023

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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:

  • the first row corresponds to this check: flags & ~(XKB_COMPOSE_COMPILE_NO_FLAGS)
  • the second row corresponds to this check: table (i.e., the operand of the NotExpr)
  • the third row corresponds to this check: !table (i.e., the NotExpr)

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
Copy link
Author

tardigrade-9 commented Jan 17, 2024

@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
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants