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

Patch curl to fix CVE-2023-38545. #4408

Merged
merged 1 commit into from
Oct 12, 2023
Merged

Conversation

teo-tsirpanis
Copy link
Member

@teo-tsirpanis teo-tsirpanis commented Oct 11, 2023

SC-35356

Fixes CVE-2023-38545


TYPE: IMPROVEMENT
DESC: Patch curl to fix CVE-2023-38545.

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #35356: Update curl to version 8.4.0 to fix CVE-2023-38545.

@KiterLuc KiterLuc requested a review from davisp October 11, 2023 13:11
@teo-tsirpanis
Copy link
Member Author

Because of breaking build system changes this new curl version introduced, I kept using version 7.88.1, and patched it with an officially provided patch.

@teo-tsirpanis teo-tsirpanis changed the title Update curl to version 8.4.0. Patch curl to fix CVE-2023-38545. Oct 11, 2023
Copy link
Contributor

@davisp davisp left a comment

Choose a reason for hiding this comment

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

+1. Verified the patch matches.

@KiterLuc KiterLuc merged commit 954509f into TileDB-Inc:dev Oct 12, 2023
51 checks passed
@teo-tsirpanis teo-tsirpanis deleted the curl-update branch October 12, 2023 09:28
KiterLuc pushed a commit that referenced this pull request Feb 5, 2024
#4528 introduced a single `TileDB::tiledb`exported CMake target for
TileDB with either static or dynamic linkage. For compatibility with
previous versions, the targets `TileDB::tiledb_shared` or
`TileDB::tiledb_static` were also defined depending on the linkage, as
`ALIAS`es to `TileDB::tiledb`.

As it turns out however, we cannot use `ALIAS` targets, because they are
always declared in the global scope prior to CMake 3.18 and if
`find_package(TileDB)` is not called in the top-level `CMakeLists.txt`
file, it will fail with `add_library cannot create ALIAS target
"TileDB::tiledb_shared" because target "TileDB::tiledb" is imported but
not globally visible.`.

Nor can we switch to using `IMPORTED INTERFACE` targets and linking them
to `TileDB::tiledb`, because it would bring a minor breaking change[^1].
Because `TileDB::tiledb_shared` would become an `INTERFACE` library, it
does not have an `IMPORTED_LOCATION` anymore, which would cause [calls
to
`install_target_libs(TileDB::tiledb_shared)`](https://github.com/TileDB-Inc/TileDB-VCF/blob/5bcc79b07935ac540c56bf6ed9ee0f5d60bf247e/libtiledbvcf/cmake/Modules/FindTileDB_EP.cmake#L121)
to fail.

Thankfully there is another solution. We set the
[`EXPORT_NAME`](https://cmake.org/cmake/help/latest/prop_tgt/EXPORT_NAME.html)
of the `tiledb` target to either `tiledb_shared` or `tiledb_static`
depending on the linkage, and define `TileDB::tiledb` as an `IMPORTED
INTERFACE` target[^2] to the linkage-specific target. This maintains
full compatibility.

[^1]: Something similar is the "breaking build system change" I talked
about in
#4408 (comment).
After removing the `install_target_libs` calls from this repository, the
change in Curl did not afffect us and we could update much more easily.
[^2]: In this opposite case the unified target _must_ be an `IMPORTED
INTERFACE`. We cannot get the `IMPORTED_LOCATION` of `TileDB::tiledb`,
but since the target is new this is not a breaking change.

---
TYPE: BUILD
DESC: Fix importing TileDB in CMake versions prior to 3.18.
github-actions bot pushed a commit that referenced this pull request Feb 5, 2024
#4528 introduced a single `TileDB::tiledb`exported CMake target for
TileDB with either static or dynamic linkage. For compatibility with
previous versions, the targets `TileDB::tiledb_shared` or
`TileDB::tiledb_static` were also defined depending on the linkage, as
`ALIAS`es to `TileDB::tiledb`.

As it turns out however, we cannot use `ALIAS` targets, because they are
always declared in the global scope prior to CMake 3.18 and if
`find_package(TileDB)` is not called in the top-level `CMakeLists.txt`
file, it will fail with `add_library cannot create ALIAS target
"TileDB::tiledb_shared" because target "TileDB::tiledb" is imported but
not globally visible.`.

Nor can we switch to using `IMPORTED INTERFACE` targets and linking them
to `TileDB::tiledb`, because it would bring a minor breaking change[^1].
Because `TileDB::tiledb_shared` would become an `INTERFACE` library, it
does not have an `IMPORTED_LOCATION` anymore, which would cause [calls
to
`install_target_libs(TileDB::tiledb_shared)`](https://github.com/TileDB-Inc/TileDB-VCF/blob/5bcc79b07935ac540c56bf6ed9ee0f5d60bf247e/libtiledbvcf/cmake/Modules/FindTileDB_EP.cmake#L121)
to fail.

Thankfully there is another solution. We set the
[`EXPORT_NAME`](https://cmake.org/cmake/help/latest/prop_tgt/EXPORT_NAME.html)
of the `tiledb` target to either `tiledb_shared` or `tiledb_static`
depending on the linkage, and define `TileDB::tiledb` as an `IMPORTED
INTERFACE` target[^2] to the linkage-specific target. This maintains
full compatibility.

[^1]: Something similar is the "breaking build system change" I talked
about in
#4408 (comment).
After removing the `install_target_libs` calls from this repository, the
change in Curl did not afffect us and we could update much more easily.
[^2]: In this opposite case the unified target _must_ be an `IMPORTED
INTERFACE`. We cannot get the `IMPORTED_LOCATION` of `TileDB::tiledb`,
but since the target is new this is not a breaking change.

---
TYPE: BUILD
DESC: Fix importing TileDB in CMake versions prior to 3.18.

(cherry picked from commit ad0371f)
teo-tsirpanis added a commit that referenced this pull request Feb 5, 2024
#4528 introduced a single `TileDB::tiledb`exported CMake target for
TileDB with either static or dynamic linkage. For compatibility with
previous versions, the targets `TileDB::tiledb_shared` or
`TileDB::tiledb_static` were also defined depending on the linkage, as
`ALIAS`es to `TileDB::tiledb`.

As it turns out however, we cannot use `ALIAS` targets, because they are
always declared in the global scope prior to CMake 3.18 and if
`find_package(TileDB)` is not called in the top-level `CMakeLists.txt`
file, it will fail with `add_library cannot create ALIAS target
"TileDB::tiledb_shared" because target "TileDB::tiledb" is imported but
not globally visible.`.

Nor can we switch to using `IMPORTED INTERFACE` targets and linking them
to `TileDB::tiledb`, because it would bring a minor breaking change[^1].
Because `TileDB::tiledb_shared` would become an `INTERFACE` library, it
does not have an `IMPORTED_LOCATION` anymore, which would cause [calls
to
`install_target_libs(TileDB::tiledb_shared)`](https://github.com/TileDB-Inc/TileDB-VCF/blob/5bcc79b07935ac540c56bf6ed9ee0f5d60bf247e/libtiledbvcf/cmake/Modules/FindTileDB_EP.cmake#L121)
to fail.

Thankfully there is another solution. We set the
[`EXPORT_NAME`](https://cmake.org/cmake/help/latest/prop_tgt/EXPORT_NAME.html)
of the `tiledb` target to either `tiledb_shared` or `tiledb_static`
depending on the linkage, and define `TileDB::tiledb` as an `IMPORTED
INTERFACE` target[^2] to the linkage-specific target. This maintains
full compatibility.

[^1]: Something similar is the "breaking build system change" I talked
about in
#4408 (comment).
After removing the `install_target_libs` calls from this repository, the
change in Curl did not afffect us and we could update much more easily.
[^2]: In this opposite case the unified target _must_ be an `IMPORTED
INTERFACE`. We cannot get the `IMPORTED_LOCATION` of `TileDB::tiledb`,
but since the target is new this is not a breaking change.

---
TYPE: BUILD
DESC: Fix importing TileDB in CMake versions prior to 3.18.

(cherry picked from commit ad0371f)
github-actions bot pushed a commit that referenced this pull request Feb 5, 2024
#4528 introduced a single `TileDB::tiledb`exported CMake target for
TileDB with either static or dynamic linkage. For compatibility with
previous versions, the targets `TileDB::tiledb_shared` or
`TileDB::tiledb_static` were also defined depending on the linkage, as
`ALIAS`es to `TileDB::tiledb`.

As it turns out however, we cannot use `ALIAS` targets, because they are
always declared in the global scope prior to CMake 3.18 and if
`find_package(TileDB)` is not called in the top-level `CMakeLists.txt`
file, it will fail with `add_library cannot create ALIAS target
"TileDB::tiledb_shared" because target "TileDB::tiledb" is imported but
not globally visible.`.

Nor can we switch to using `IMPORTED INTERFACE` targets and linking them
to `TileDB::tiledb`, because it would bring a minor breaking change[^1].
Because `TileDB::tiledb_shared` would become an `INTERFACE` library, it
does not have an `IMPORTED_LOCATION` anymore, which would cause [calls
to
`install_target_libs(TileDB::tiledb_shared)`](https://github.com/TileDB-Inc/TileDB-VCF/blob/5bcc79b07935ac540c56bf6ed9ee0f5d60bf247e/libtiledbvcf/cmake/Modules/FindTileDB_EP.cmake#L121)
to fail.

Thankfully there is another solution. We set the
[`EXPORT_NAME`](https://cmake.org/cmake/help/latest/prop_tgt/EXPORT_NAME.html)
of the `tiledb` target to either `tiledb_shared` or `tiledb_static`
depending on the linkage, and define `TileDB::tiledb` as an `IMPORTED
INTERFACE` target[^2] to the linkage-specific target. This maintains
full compatibility.

[^1]: Something similar is the "breaking build system change" I talked
about in
#4408 (comment).
After removing the `install_target_libs` calls from this repository, the
change in Curl did not afffect us and we could update much more easily.
[^2]: In this opposite case the unified target _must_ be an `IMPORTED
INTERFACE`. We cannot get the `IMPORTED_LOCATION` of `TileDB::tiledb`,
but since the target is new this is not a breaking change.

---
TYPE: BUILD
DESC: Fix importing TileDB in CMake versions prior to 3.18.

(cherry picked from commit ad0371f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants