Skip to content

Conversation

@MaxKsyunz
Copy link
Collaborator

@MaxKsyunz MaxKsyunz commented May 30, 2023

CVE is here.

This CVE does not affect deployments of sql plugin because only integration tests use sqlite-jdbc.

Issues Resolved

#1669

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Yury-Fridlyand
Yury-Fridlyand previously approved these changes May 30, 2023
@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #1667 (4e51282) into main (c7dfdb3) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #1667   +/-   ##
=========================================
  Coverage     97.29%   97.29%           
  Complexity     4408     4408           
=========================================
  Files           388      388           
  Lines         10944    10944           
  Branches        774      774           
=========================================
  Hits          10648    10648           
  Misses          289      289           
  Partials          7        7           
Flag Coverage Δ
sql-engine 97.29% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@MaxKsyunz
Copy link
Collaborator Author

This introduced a proper CI failure 😱 . CorrectnessIT is failing for three test cases. Here's one of them:

 {
      "explain": "Schema type at [0] is different: this=[Type(name=-0.36651292058166435, type=[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT])], other=[Type(name=LOG(LOG(2)), type=[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT])], Schema type at [0] is different: this=[Type(name=-0.36651292058166435, type=[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT])], other=[Type(name=LOG(LOG(2)), type=[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT])]",
      "result": "Failed",
      "resultSets": [
        {
          "schema": [{
            "name": "-0.36651292058166435",
            "type": "[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT]"
          }],
          "dataRows": [[-0.36]],
          "database": "H2"
        },
        {
          "schema": [{
            "name": "LOG(LOG(2))",
            "type": "[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT]"
          }],
          "dataRows": [[-0.36]],
          "database": "OpenSearch"
        },
        {
          "schema": [{
            "name": "LOG(LOG(2))",
            "type": "[FLOAT, DOUBLE, REAL, DOUBLE PRECISION, DECFLOAT]"
          }],
          "dataRows": [[-0.52]],
          "database": "SQLite"
        }
      ],
      "id": 98,
      "errors": "",
      "sql": "SELECT log(log(2))"
    },

acarbonetto
acarbonetto previously approved these changes May 30, 2023
@Yury-Fridlyand Yury-Fridlyand linked an issue May 30, 2023 that may be closed by this pull request
1 task
penghuo
penghuo previously approved these changes Jun 7, 2023
@Yury-Fridlyand Yury-Fridlyand mentioned this pull request Jun 12, 2023
6 tasks
@rupal-bq
Copy link
Contributor

@MaxKsyunz Can you look into failed test? If there's any blocker then we can check adding exception for this.

@Yury-Fridlyand
Copy link
Collaborator

Why it started to fail?

1
xerial/sqlite-jdbc#763
In the updated version, the default evaluation of log(x) has changed from ln(x) to log10(x), which is a breaking change.
Thanks @vamsi-amazon for finding this.

2
H2 DB returns valid result, but it changes column name. For example, for query select 1 + 1 H2 returns a result set with a column named 2.

3
Correctness test compares result (DBresult object) for every query gotten from OpenSearch with result from H2 and SQLite. If comparison with both references fails => query check fails => all test fails.

4
All queries with logarithms produced incorrect value on SQLite due to changes described in 1 and incorrect column name on H2 as described in 2. All queries with log call failed.

Fix

Don't validate column name on H2 results.

@Yury-Fridlyand Yury-Fridlyand dismissed stale reviews from penghuo, acarbonetto, and themself via 62bc46b June 23, 2023 04:16
acarbonetto
acarbonetto previously approved these changes Jun 23, 2023
Signed-off-by: Yury-Fridlyand <[email protected]>
Copy link
Collaborator

@dai-chen dai-chen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@dai-chen dai-chen merged commit dc4e468 into opensearch-project:main Jun 26, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 1.3 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-1667-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 dc4e468a4849d6f2fd28b15d1b926d8cce808a9f
# Push it to GitHub
git push --set-upstream origin backport/backport-1667-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

Then, create a pull request where the base branch is 1.3 and the compare/head branch is backport/backport-1667-to-1.3.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1667-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 dc4e468a4849d6f2fd28b15d1b926d8cce808a9f
# Push it to GitHub
git push --set-upstream origin backport/backport-1667-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1667-to-2.x.

@Yury-Fridlyand Yury-Fridlyand deleted the chore/cve-2023-32697 branch June 27, 2023 00:51
Yury-Fridlyand added a commit to Bit-Quill/opensearch-project-sql that referenced this pull request Jun 27, 2023
…project#1667)

* Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697

Signed-off-by: MaxKsyunz <[email protected]>

* Don't check column names on H2 results for correctness tests as described in opensearch-project#1667 (comment).

Signed-off-by: Yury-Fridlyand <[email protected]>

* Address PR review comment.

Signed-off-by: Yury-Fridlyand <[email protected]>

---------

Signed-off-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Yury-Fridlyand added a commit that referenced this pull request Jun 27, 2023
* Update SQL plugin for core refactor (#1571)

Signed-off-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>

* Fix plugin compilation (#1580)

* Changed gradle version and removed values iterator

Signed-off-by: Guian Gumpac <[email protected]>

* Update a test to match new indexResponse.aliases() type.

Signed-off-by: MaxKsyunz <[email protected]>

* Ran ./gradlew wrapper

Signed-off-by: Guian Gumpac <[email protected]>

---------

Signed-off-by: Guian Gumpac <[email protected]>
Signed-off-by: MaxKsyunz <[email protected]>
Co-authored-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>

* Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697 (#1667)

* Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697

Signed-off-by: MaxKsyunz <[email protected]>

* Don't check column names on H2 results for correctness tests as described in #1667 (comment).

Signed-off-by: Yury-Fridlyand <[email protected]>

* Address PR review comment.

Signed-off-by: Yury-Fridlyand <[email protected]>

---------

Signed-off-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>

---------

Signed-off-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Guian Gumpac <[email protected]>
Co-authored-by: MaxKsyunz <[email protected]>
@derek-ho derek-ho mentioned this pull request Aug 4, 2023
6 tasks
derek-ho pushed a commit to derek-ho/sql that referenced this pull request Aug 4, 2023
…project#1667)

* Update sqlite-jdbc to 3.41.2.2 to address CVE-2023-32697

Signed-off-by: MaxKsyunz <[email protected]>

* Don't check column names on H2 results for correctness tests as described in opensearch-project#1667 (comment).

Signed-off-by: Yury-Fridlyand <[email protected]>

* Address PR review comment.

Signed-off-by: Yury-Fridlyand <[email protected]>

---------

Signed-off-by: MaxKsyunz <[email protected]>
Signed-off-by: Yury-Fridlyand <[email protected]>
Co-authored-by: Yury-Fridlyand <[email protected]>
Signed-off-by: Derek Ho <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CVE-2023-32697 (Critical) detected in sqlite-jdbc-3.32.3.3.jar

8 participants