[ 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)