You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2023/01/05 12:14:44 UTC
[GitHub] [logging-log4j2] whiteChen233 opened a new pull request, #1192: fix(sec): upgrade org.slf4j:slf4j-ext to 1.7.26
whiteChen233 opened a new pull request, #1192:
URL: https://github.com/apache/logging-log4j2/pull/1192
### What happened？
There are 1 security vulnerabilities found in org.slf4j:slf4j-ext 1.7.25
- [CVE-2018-8088](https://www.oscs1024.com/hd/CVE-2018-8088)
### What did I do？
Upgrade org.slf4j:slf4j-ext from 1.7.25 to 1.7.26 for vulnerability fix
### What did you expect to happen？
Ideally, no insecure libs should be used.
### How was this patch tested?
Run `mvn compile` failed locally, couldn't complete the build process.
Run `mvn clean test` failed locally, unit-test couldn't pass.
### The specification of the pull request
[PR Specification](https://www.oscs1024.com/docs/pr-specification/) from OSCS
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] vy commented on pull request #1192: fix(sec): upgrade org.slf4j:slf4j-ext to 1.7.26
Posted by GitBox <gi...@apache.org>.
vy commented on PR #1192:
URL: https://github.com/apache/logging-log4j2/pull/1192#issuecomment-1373571950
@ppkarwasz, it should be possible, but I guess, it is not. :confounded: I have already wasted hours on this, hence I will not go into trouble of explaining it, though I will share my attempts:
## Using `build-helper-maven-plugin` to set `slf4jExtVersion` property
```xml
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>build-helper-maven-plugin</artifactId>
<version>3.3.0</version>
<executions>
<execution>
<id>replace-slf4j-version-property</id>
<phase>prepare-package</phase>
<goals>
<goal>bsh-property</goal>
</goals>
<configuration>
<properties>
<property>slf4jExtVersion</property>
</properties>
<source>
slf4jExtVersion = "1.7.36";
</source>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>flatten-maven-plugin</artifactId>
<version>1.3.0</version>
<inherited>false</inherited>
<executions>
<execution>
<id>flatten-bom</id>
<phase>prepare-package</phase>
<goals>
<goal>flatten</goal>
</goals>
<configuration>
<flattenMode>clean</flattenMode>
<!-- POM `ElementHandling` is pretty cryptic: https://www.mojohaus.org/flatten-maven-plugin/apidocs/org/codehaus/mojo/flatten/ElementHandling.html
Trial-and-error has shown that we should use either `remove` or `interpolate`.
`remove` simply removes the element.
`interpolate` takes the element from the original POM with variables interpolated.
Avoid using `resolve`, which uses the effective POM where inherited changes from the parent are also incorporated. -->
<pomElements>
<build>remove</build>
<properties>remove</properties>
<repositories>remove</repositories>
<distributionManagement>remove</distributionManagement>
<dependencyManagement>interpolate</dependencyManagement>
</pomElements>
<!-- Setting `outputDirectory` to `project.build.directory`, which is cleaned by `default-clean` execution of `clean:clean`.
This makes `flatten:clean` redundant. -->
<outputDirectory>${project.build.directory}</outputDirectory>
</configuration>
</execution>
</executions>
</plugin>
```
This doesn't work. `flatten-maven-plugin` doesn't use the version set by the `build-helper-maven-plugin`. I suppose it simply reads the `pom.xml` ignoring the current Maven context.
## Using `versions-maven-plugin` to set `slf4jExtVersion` property
```xml
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>versions-maven-plugin</artifactId>
<version>2.14.2</version>
<executions>
<execution>
<id>replace-slf4j-version-property</id>
<phase>prepare-package</phase>
<goals>
<goal>set-property</goal>
</goals>
<configuration>
<property>slf4jExtVersion</property>
<newVersion>1.7.36</newVersion>
</configuration>
</execution>
<execution>
<id>revert-slf4j-version-property</id>
<phase>package</phase>
<goals>
<goal>set-property</goal>
</goals>
<configuration>
<property>slf4jExtVersion</property>
<newVersion>1.7.25</newVersion>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>flatten-maven-plugin</artifactId>
<!-- same as previous usage -->
</plugin>
```
**This works!** But... `replace-slf4j-version-property` updates the original `pom.xml`, though `revert-slf4j-version-property` doesn't revert it back. Hence when `./mvnw package` finishes, it leaves the repository in an unclean state: `pom.xml` contains `<slf4jExtVersion>1.7.36</slf4jExtVersion>` change, whereas it should have been left `<slf4jExtVersion>1.7.25</slf4jExtVersion>`.
## Conclusion
`versions-maven-plugin` method can be used. But ideally `flatten-maven-plugin` should be updated to accept a list of properties to take into account while interpolating property substitutions. All of this is very ugly, including the original problem itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] ppkarwasz commented on pull request #1192: fix(sec): upgrade org.slf4j:slf4j-ext to 1.7.26
Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on PR #1192:
URL: https://github.com/apache/logging-log4j2/pull/1192#issuecomment-1372186537
@whiteChen233,
As you can see from the comment:
> Do not upgrade the SLF4J version. 1.7.26 broke backward compatibility. Users can update the version if they do not require support for SLF4J's `EventData`.
we intentionally maintain backward compatibility with `slf4j-ext` version 1.7.25. It is up to the user to choose the version of `slf4j-ext` he wishes to use. If he uses `EventData`, `log4j-slf4j-impl` has support for it.
Personally I would accept a PR that uses `sl4f-ext` 1.7.25 to compile the artifact, but bumps the version in the published POM. Basically we **need** 1.7.25 to compile the artifact, but we wish version 1.7.36 to appear as the **recommended** version of `slf4j-ext`. @vy, any idea which Maven plugin can do this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(sec): upgrade org.slf4j:slf4j-ext to 1.7.26 (logging-log4j2)
Posted by "vy (via GitHub)" <gi...@apache.org>.
vy closed pull request #1192: fix(sec): upgrade org.slf4j:slf4j-ext to 1.7.26
URL: https://github.com/apache/logging-log4j2/pull/1192
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org