You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/06/16 01:17:31 UTC

[GitHub] [trafficserver] masaori335 opened a new pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

masaori335 opened a new pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945


   We got an ASan report on 8.1.2. It looks like the temporal buffer on the stack is not enough to escape the URL.
   ```
    ==37529==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7f7c922ec824 at pc 0x55fc2573e23a bp 0x7f7c922ec570 sp 0x7f7c922ec568
    WRITE of size 1 at 0x7f7c922ec824 thread T3 ([ET_NET 1])
       #0 0x55fc2573e239 in _ZN12_GLOBAL__N_119escapify_url_commonEP5ArenaPcmPiS2_mPKhb proxy/logging/LogUtils.cc:393:7
       #1 0x55fc2573e34d in _ZN8LogUtils17pure_escapify_urlEP5ArenaPcmPiS2_mPKh proxy/logging/LogUtils.cc:410:10
       #2 0x55fc2544eb34 in TSStringPercentEncode src/traffic_server/InkAPI.cc:2372:18
       #3 0x7f7c4ece0529 in _ZL13appendEncodedRNSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEEPKcm plugins/cachekey/cachekey.cc:70:7
       #4 0x7f7c4ece33a7 in _ZN8CacheKey6appendERKNSt3__112basic_stringIcNS0_11char_traitsIcEENS0_9allocatorIcEEEE plugins/cachekey/cachekey.cc:334:3
       #5 0x7f7c4ece33a7 in _ZN8CacheKey10appendPathER7PatternS1_ plugins/cachekey/cachekey.cc:481:5
       ...
   ```


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] masaori335 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865423197


   Oops, I close mine (#7972)


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] ezelkow1 edited a comment on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
ezelkow1 edited a comment on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865402604


   ill just bring this into a larger fix up pr, https://github.com/apache/trafficserver/pull/7971


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] ezelkow1 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865252888


   Can we get a backport PR for 8.1.x?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] zwoop commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865223809


   Cherry-picked to v9.0.x branch.
   Cherry-picked to v9.1.x branch.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] masaori335 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-862074440


   Good point. The `escapify_url_common()`, core function called by `TSStringPercentEncode()`, is checking the `dst_size` but not include null termination. This is why we got `dynamic-stack-buffer-overflow` on line 410.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] zwoop commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865223809


   Cherry-picked to v9.0.x branch.
   Cherry-picked to v9.1.x branch.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] ezelkow1 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865402604


   ill just bring this into a larger fix up pr


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] masaori335 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865423197


   Oops, I close mine (#7972)


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
maskit commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-861961761


   The fix for cachmekey plugin is reasonable, but I think actual bug is in `TSStringPercentEncode`. It receives `dst_size` so it should not crash even if a destination buffer doesn't have enough size.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] ezelkow1 edited a comment on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
ezelkow1 edited a comment on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865402604


   ill just bring this into a larger fix up pr, https://github.com/apache/trafficserver/pull/7971


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] maskit edited a comment on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
maskit edited a comment on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-861961761


   The fix for cachekey plugin is reasonable, but I think actual bug is in `TSStringPercentEncode`. It receives `dst_size` so it should not crash even if a destination buffer doesn't have enough size.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [trafficserver] ezelkow1 commented on pull request #7945: Fix dynamic-stack-buffer-overflow of cachekey plugin

Posted by GitBox <gi...@apache.org>.
ezelkow1 commented on pull request #7945:
URL: https://github.com/apache/trafficserver/pull/7945#issuecomment-865252888






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org