| Summary: | Suggestion for code improvement (avoiding potential bug) | ||
|---|---|---|---|
| Product: | Tomcat 8 | Reporter: | Dongcai Shen <shensiulap> |
| Component: | Connectors | Assignee: | Tomcat Developers Mailing List <dev> |
| Status: | RESOLVED FIXED | ||
| Severity: | enhancement | ||
| Priority: | P2 | ||
| Version: | 8.0.x-trunk | ||
| Target Milestone: | ---- | ||
| Hardware: | PC | ||
| OS: | Linux | ||
Tomcat 5.5.x is no longer supported. Looking at the mod_jk, I don't see how there could ever be a problem here but there is scope for removing the additional unnecessary calls. Moving this to Tomcat 8.0.x and marking as an enhancement. This will most likely get be applied to 8.0.x only, as part of the ongoing clean-up of the code base for the 8.0.x branch. Fixed in 8.0.x and will be included in 8.0.0 onwards. Thank Mark for your efforts. The credit of this vulnerability should be given to this bug report: https://issues.apache.org/bugzilla/show_bug.cgi?id=36883 They discovered the bug but only fixed in the httpd site and left the problem in tomcat intact. Now the problem at the tomcat part is also fixed. |
In connectors/jk/java/org/apache/jk/common/HandlerRequest.java coyote.Request's schemeMB is assigned in 2 places. 1st place: 400 boolean isSSL = msg.getByte() != 0; 401 if( isSSL ) { 402 // XXX req.setSecure( true ); 403 req.scheme().setString("https"); 404 } 2nd place: 518 case AjpConstants.SC_A_SSL_CERT : 519 req.scheme().setString( "https" ); and similar assignments for SC_A_SSL_CIPHER and SC_A_SSL_SESSION cases below. It seems they do not make sense because the packet's 8-bit field is designated for telling whether it's SSL or not. So the 1st place is enough. Adding the 2nd place may pose potential bug in that a packet with the 8-bit SSL field being 0 and suffixes of SC_A_SSL_* key-value pairs can later incorrect trigger a wrong redirection message pointing to a https location. A simple correction is to honor the 8-bit SSL-field in packet and delete the 3 lines of 2nd place assigning "https". Even though the chances of such spurious packet is low, but it's best we can have threat-free, semantic-correct tomcat code. The same lines of code remain in 6.0 and 7.0. But maybe I misunderstand the code, in which case please kindly point out. Thanks.