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.

Sign up for GitHub

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

Jump to bottom

CsrfWebFilter null save content-type check #11204

Closed
ZhivkoDelchev opened this issue May 12, 2022 · 1 comment · Fixed by #11205
Closed

CsrfWebFilter null save content-type check #11204

ZhivkoDelchev opened this issue May 12, 2022 · 1 comment · Fixed by #11205
Assignees
@rwinch
Labels
status: backported An issue that has been backported to maintenance branches status: duplicate A duplicate of another issue type: bug A general bug

Comments

@ZhivkoDelchev
Copy link
Contributor

ZhivkoDelchev commented May 12, 2022 •
edited
Loading

Bug description
Performing non-GET requests with no content-type header results in a NullPointerException from CsrfWebFilter when MultipartFormData is enabled.

To Reproduce
Perform non-GET request with no body & content-type header with MultipartFormData enabled.

Expected behavior
All methods without body & content-type header should work.

Stacktrace

500 Server Error for HTTP DELETE "...."
java.lang.NullPointerException: null
	at org.springframework.security.web.server.csrf.CsrfWebFilter.tokenFromMultipartData(CsrfWebFilter.java:154)
	Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
	*__checkpoint ⇢ org.springframework.security.web.server.csrf.CsrfWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.header.HttpHeaderWriterWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.config.web.server.ServerHttpSecurity$ServerWebExchangeReactorContextWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.security.web.server.WebFilterChainProxy [DefaultWebFilterChain]
	*__checkpoint ⇢ org.springframework.boot.actuate.metrics.web.reactive.server.MetricsWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ com.tis.commons.traceid.reactive.TraceIdWebFilter [DefaultWebFilterChain]
	*__checkpoint ⇢ HTTP DELETE "...." [ExceptionHandlingWebHandler]
Original Stack Trace:
		at org.springframework.security.web.server.csrf.CsrfWebFilter.tokenFromMultipartData(CsrfWebFilter.java:154)
		at org.springframework.security.web.server.csrf.CsrfWebFilter.containsValidCsrfToken(CsrfWebFilter.java:143)
		at org.springframework.security.web.server.csrf.CsrfWebFilter.lambda$validateToken$5(CsrfWebFilter.java:136)
		at reactor.netty.http.server.HttpServer$HttpServerHandle.onStateChange(HttpServer.java:967) [113 skipped]
		at reactor.netty.ReactorNetty$CompositeConnectionObserver.onStateChange(ReactorNetty.java:677)
		at reactor.netty.transport.ServerTransport$ChildObserver.onStateChange(ServerTransport.java:478)
		at reactor.netty.http.server.HttpServerOperations.onInboundNext(HttpServerOperations.java:570)
		at reactor.netty.channel.ChannelOperationsHandler.channelRead(ChannelOperationsHandler.java:93)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
		at reactor.netty.http.server.HttpTrafficHandler.channelRead(HttpTrafficHandler.java:222)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
		at io.netty.channel.CombinedChannelDuplexHandler$DelegatingChannelHandlerContext.fireChannelRead(CombinedChannelDuplexHandler.java:436)
		at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:327)
		at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:299)
		at io.netty.channel.CombinedChannelDuplexHandler.channelRead(CombinedChannelDuplexHandler.java:251)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
		at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1372)
		at io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1235)
		at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1284)
		at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:510)
		at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:449)
		at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:279)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
		at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357)
		at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379)
		at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365)
		at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
		at io.netty.channel.epoll.AbstractEpollStreamChannel$EpollStreamUnsafe.epollInReady(AbstractEpollStreamChannel.java:800)
		at io.netty.channel.epoll.AbstractEpollChannel$AbstractEpollUnsafe$1.run(AbstractEpollChannel.java:425)
		at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
		at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469)
		at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:384)
		at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:986)
		at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
		at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
		at java.base\/java.lang.Thread.run(Thread.java:829)
@ZhivkoDelchev ZhivkoDelchev added status: waiting-for-triage An issue we've not yet triaged type: bug A general bug labels May 12, 2022
@ZhivkoDelchev ZhivkoDelchev mentioned this issue May 12, 2022
Merged
@rwinch rwinch added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels May 17, 2022
@rwinch
Copy link
Member

rwinch commented May 17, 2022

Thanks for the report and the PR! Closing in favor of gh-11205

@rwinch rwinch closed this as completed May 17, 2022
@rwinch rwinch added status: duplicate A duplicate of another issue and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 2, 2022
@rwinch rwinch self-assigned this Jun 2, 2022
@rwinch rwinch changed the title NPE on requests with no content-type header with MultipartFormData enabled CsrfWebFilter null save content-type check Jun 2, 2022
ZhivkoDelchev added a commit to ZhivkoDelchev/spring-security that referenced this issue Jun 5, 2022
@ZhivkoDelchev
Reverse content type check
c4e286e 
When MultipartFormData is enabled currently the CsrfWebFilter compares
the content-type header against MULTIPART_FORM_DATA MediaType which
leads to NullPointerExecption when there is no content-type header.
This commit reverse the check to compare the MULTIPART_FORM_DATA
MediaType against the content-type which contains null check and avoids
the exception.

closes spring-projectsgh-11204
rwinch pushed a commit that referenced this issue Jun 6, 2022
@ZhivkoDelchev @rwinch
Reverse content type check
1483a57 
When MultipartFormData is enabled currently the CsrfWebFilter compares
the content-type header against MULTIPART_FORM_DATA MediaType which
leads to NullPointerExecption when there is no content-type header.
This commit reverse the check to compare the MULTIPART_FORM_DATA
MediaType against the content-type which contains null check and avoids
the exception.

closes gh-11204
rwinch pushed a commit that referenced this issue Jun 6, 2022
@ZhivkoDelchev @rwinch
Reverse content type check
cf69cdf 
When MultipartFormData is enabled currently the CsrfWebFilter compares
the content-type header against MULTIPART_FORM_DATA MediaType which
leads to NullPointerExecption when there is no content-type header.
This commit reverse the check to compare the MULTIPART_FORM_DATA
MediaType against the content-type which contains null check and avoids
the exception.

closes gh-11204
Closes gh-11205
rwinch pushed a commit that referenced this issue Jun 6, 2022
@ZhivkoDelchev @rwinch
Reverse content type check
d882bfc 
When MultipartFormData is enabled currently the CsrfWebFilter compares
the content-type header against MULTIPART_FORM_DATA MediaType which
leads to NullPointerExecption when there is no content-type header.
This commit reverse the check to compare the MULTIPART_FORM_DATA
MediaType against the content-type which contains null check and avoids
the exception.

closes gh-11204
Closes gh-11205
rwinch pushed a commit that referenced this issue Jun 6, 2022
@ZhivkoDelchev @rwinch
Reverse content type check
e97c5a5 
When MultipartFormData is enabled currently the CsrfWebFilter compares
the content-type header against MULTIPART_FORM_DATA MediaType which
leads to NullPointerExecption when there is no content-type header.
This commit reverse the check to compare the MULTIPART_FORM_DATA
MediaType against the content-type which contains null check and avoids
the exception.

closes gh-11204
Closes gh-11205
@github-actions github-actions bot added the status: backported An issue that has been backported to maintenance branches label Jun 6, 2022
This was referenced Jun 6, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rwinch rwinch

Labels
status: backported An issue that has been backported to maintenance branches status: duplicate A duplicate of another issue type: bug A general bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CsrfWebFilter null save content-type check ZhivkoDelchev/spring-security
2 participants
@rwinch @ZhivkoDelchev