[ 
https://issues.apache.org/jira/browse/TS-3305?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14282133#comment-14282133
 ] 

Changming Sun commented on TS-3305:
-----------------------------------

I just find that sprintf/vsprintf is widely used in ATS. There are about 80 
places needed to fix.  I would like to fix all of them, but the patch will be 
large. Bryan Call has already fired an issue for it 3 years ago: 
https://issues.apache.org/jira/browse/TS-898

./lib/ts/ink_res_mkquery.cc:#define SPRINTF(x) (sprintf x)
./lib/ts/ink_res_mkquery.cc:    i = SPRINTF((dn, "\\[x"));
./lib/ts/ink_res_mkquery.cc:            i = SPRINTF((dn, "%02x", *cp & 0xff));
./lib/ts/ink_res_mkquery.cc:            i = SPRINTF((dn, "%02x", tc & (0xff << 
(8 - b))));
./lib/ts/ink_res_mkquery.cc:            i = SPRINTF((dn, "%1x",
./lib/ts/ink_res_mkquery.cc:    i = SPRINTF((dn, "/%d]", blen));
./lib/wccp/WccpStatic.cc:  sprintf(buff, "%d.%d.%d.%d", octet[0], octet[1], 
octet[2], octet[3]);
./lib/records/P_RecCore.cc:  sprintf(tmp_filename, "%s%s", g_rec_config_fpath, 
TMP_FILENAME_EXT_STR);
./lib/ts/ink_res_init.cc:          sprintf(sbuf, "%d", NAMESERVER_PORT);
./lib/ts/ink_code.cc:    sprintf(&(dest33[i * 2]), "%02X", md5[i]);
./lib/ts/ink_res_mkquery.cc:#define SPRINTF(x) (sprintf x)
./doc/sdk/sample-source-code.en.rst:           sprintf (buf, "You are forbidden 
from accessing \"%s\"\n", url_str);
./doc/sdk/sample-source-code.en.rst:           sprintf (blacklist_file, 
"%s/blacklist.txt", TSPluginDirGet());
./plugins/gzip/misc.cc:    sprintf(hidden_header_name, "x-accept-encoding-%s", 
result);
./plugins/experimental/ts_lua/ts_lua.c:    sprintf(conf->script, "%s", argv[2]);
./plugins/experimental/ts_lua/ts_lua.c:  sprintf(conf->script, "%s", argv[1]);
./plugins/experimental/esi/combo_handler.cc:          int line_size = 
sprintf(line_buf, "Cache-Control: max-age=%d, public\r\n", max_age);
./plugins/experimental/esi/fetcher/HttpDataFetcherImpl.cc:  sprintf(http_req, 
"GET %s HTTP/1.0\r\n%s\r\n", url.c_str(), _headers_str.c_str());
./plugins/experimental/buffer_upload/buffer_upload.cc:      sprintf(path, 
"%s/%02X/tmp-XXXXXX", uconfig->base_dir, index);
./plugins/experimental/buffer_upload/buffer_upload.cc:    
sprintf(default_filename, "%s/upload.conf", TSPluginDirGet());
./plugins/experimental/url_sig/url_sig.c:    sprintf(&(sig_string[i * 2]), 
"%02x", sig[i]);
./plugins/experimental/mysql_remap/lib/dictionary.c:            sprintf(cval, 
"%04d", i);
./plugins/experimental/mysql_remap/lib/dictionary.c:            sprintf(cval, 
"%04d", i);
./plugins/experimental/mysql_remap/lib/dictionary.c:            sprintf(cval, 
"%04d", i);
./plugins/experimental/mysql_remap/lib/iniparser.c:        sprintf(keym, "%s:", 
secname);
./plugins/experimental/mysql_remap/lib/iniparser.c:            sprintf(tmp, 
"%s:%s", section, key);
./plugins/experimental/channel_stats/channel_stats.cc:    if (!sprintf(buf, 
":%d", pristine_port))
./plugins/experimental/ats_pagespeed/gzip/misc.cc:    
sprintf(hidden_header_name, "x-accept-encoding-%s", result);
./proxy/http/HttpUpdateTester.cc:    sprintf(req_buf, "GET %s 
HTTP/1.0\r\nCache-Control: max-age=0\r\n\r\n", url_buf);
./proxy/http/HttpBodyFactory.cc:      *resulting_buffer_length = 
ink_bvsprintf(buffer, format, ap) - 1;
./proxy/TestProxy.cc:    sprintf(outbuf->start, "GET %s HTTP/1.0\nHost: 
%s\n\n", url, host);
./proxy/TestProxy.cc:    sprintf(outbuf->start, "GET %s HTTP/1.0\nHost: 
%s\n\n", url, host);
./proxy/TestSimpleProxy.cc:    sprintf(outbuf->start, "GET %s HTTP/1.0\n\n\n", 
url);
./proxy/logging/Log.cc:    sprintf(desc, "[LOG_PREPROC %d]", i);
./proxy/logging/Log.cc:  sprintf(desc, "Logging flush buffer list");
./proxy/logging/Log.cc:  sprintf(desc, "[LOG_FLUSH]");
./proxy/InkAPITest.cc:  l = ink_bvsprintf(buffer, format2, ap);
./proxy/hdrs/HTTP.cc:        ui->m_len_port = sprintf(port_buff, "%.5d", 
hdr->m_port);
./proxy/hdrs/test_header.cc:    sprintf(field_name, "Test%d", i);
./proxy/hdrs/test_header.cc:    sprintf(field_value, "%d %d %d %d %d", i, i, i, 
i, i);
./proxy/hdrs/test_header.cc:    sprintf(field_name, "Test%d", i);
./proxy/hdrs/test_header.cc:    sprintf(field_name, "Test%d", i);
./proxy/hdrs/test_header.cc:    sprintf(field_value, "d %d %d %d %d %d", i, i, 
i, i, i);
./proxy/hdrs/test_header.cc:    sprintf(field_name, "Test%d", i);
./proxy/hdrs/test_header.cc:    sprintf(field_value, "a %d", i);
./proxy/hdrs/test_header.cc:    sprintf(field_name, "Test%d", i);
./example/prefetch/test-hns-plugin.c:      sprintf(file_name, 
"%s/prefetched.objects", optarg);
./example/secure-link/secure-link.c:    sprintf(&hash[i * 2], "%02x", md[i]);
./example/thread-pool/test/SDKTest/psi_server.c:      
sprintf(resp_id->header_response, "%s\r\n%s\r\n%s%ld\r\n%s\r\n\r\n",
./example/thread-pool/test/SDKTest/psi_server.c:      
sprintf(resp_id->header_response, "%s\r\n%s\r\n%s%ld\r\n\r\n",
./example/thread-pool/test/SDKTest/psi_server.c:    
sprintf(resp_id->header_response, "%s\r\n%s\r\n\r\n", "HTTP/1.0 404 Not Found", 
"Content-type: text/plain");
./example/thread-pool/test/SDKTest/psi_server.c:    i = sprintf((char *) 
resp_buffer, "%s", rid->header_response);
./example/thread-pool/test/SDKTest/psi_server.c:        len = sprintf(psi_tag, 
PSI_TAG_FORMAT, rand() % 100);
./example/thread-pool/psi.c:  sprintf(inc_file, "%s/%s", psi_directory, 
_basename(data->psi_filename));
./example/thread-pool/psi.c:  sprintf(psi_directory, "%s/%s", TSPluginDirGet(), 
PSI_PATH);
./example/thread-pool/psi.c:    sprintf(thread_name, "Thread[%d]", i);
./example/blacklist-1/blacklist-1.c:  sprintf(buf, "You are forbidden from 
accessing \"%s\"\n", url_str);
./example/blacklist-1/blacklist-1.c:  sprintf(blacklist_file, 
"%s/blacklist.txt", TSPluginDirGet());
./example/blacklist-0/blacklist-0.c:  sprintf(buf, "You are forbidden from 
accessing \"%s\"\n", url_str);
./mgmt/WebMgmtUtils.cc:  sprintf(bufVal, "%" PRId64 "", bytes);
./mgmt/WebMgmtUtils.cc:  sprintf(bufVal, "%" PRId64 "", mBytes);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:      vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:    vsprintf(message, extended_format, ap);
./mgmt/utils/MgmtUtils.cc:    vsprintf(message, extended_format, ap);
./cmd/traffic_cop/traffic_cop.cc:    vsprintf(buffer, format, args);
./cmd/traffic_top/stats.h:            sprintf(buffer, "%" PRId64, value);
./iocore/cache/CachePagesInternal.cc:    sprintf(nbytes, "%d", vc->vio.nbytes);
./iocore/cache/CachePagesInternal.cc:    sprintf(todo, "%d", vc->vio.ntodo());
./iocore/cache/CachePagesInternal.cc:      sprintf(offset, "%" PRIu64 "", 
(uint64_t) vol_offset(p, &b->dir));

> minor bugs in ats lua plugin
> ----------------------------
>
>                 Key: TS-3305
>                 URL: https://issues.apache.org/jira/browse/TS-3305
>             Project: Traffic Server
>          Issue Type: Bug
>          Components: Lua, Plugins
>            Reporter: Kit Chan
>            Assignee: Kit Chan
>             Fix For: sometime
>
>         Attachments: TS-3305.patch
>
>
> a minor bugs in ATS lua plugin, 
> In plugins/experimental/ts_lua/ts_lua.c line 89:
>   if (fn) {
>     sprintf(conf->script, "%s", argv[2]);
>   } else {
>     conf->content = argv[2];
>   }
> line: 358
>   sprintf(conf->script, "%s", argv[1]);
> "sprintf" is extremely dangerous and deprecated ,should be replaced with 
> "snprintf". 
> And it would be better ( less confused ) if a '\0' is always added to the 
> buffer after strncpy:
> e.g:
> if (argc < 3) {
>     strncpy(errbuf, "[TSRemapNewInstance] - lua script file or string is 
> required !!", errbuf_size - 1);
>     return TS_ERROR;
>   }
> Should be:
> if (argc < 3) {
>     strncpy(errbuf, "[TSRemapNewInstance] - lua script file or string is 
> required !!", errbuf_size - 1);
>     errbuf[errbuf_size - 1] = '\0';
>     return TS_ERROR;
> }
> Because strncpy doesn't guarantee null-termination. Now it works just because 
> errbuf is large enough(2048 bytes)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to