From 1dcfe72ceecafbd9d54e334383bbef39e986211e Mon Sep 17 00:00:00 2001
From: thomas <kreitler@molgen.mpg.de>
Date: Mon, 9 Oct 2017 15:33:20 +0200
Subject: [PATCH 1/3] main.cpp: Defeat impact of possible buffer overflow
'valgrind --tool=exp-sgcheck' detected an invalid write
at former line 491 (sprintf...).
Using snprintf avoids this condition, and allows data loss
detection (Q: why not use the safer c++ strings, A:???).
This fix has some observable impact on bsmap's output,
although I've only seen a change in the amount of whitespace.
I also assume that this might be the cause for the crashes
at the end of bsmap execution.
---
main.cpp | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/main.cpp b/main.cpp
index 7bab7d4..761df2d 100644
--- a/main.cpp
+++ b/main.cpp
@@ -20,6 +20,8 @@
#include <pthread.h>
#endif
+#define LINEBUFFSIZE 1024
+
using namespace std;
//global variables
@@ -397,7 +399,8 @@ int check_ifile_format(string &filename, int gz_flag) {
}
void RunProcess(void) {
- char _ch[256]; string _str="@HD\tVN:1.0\n";
+ char _ch[LINEBUFFSIZE];
+ string _str="@HD\tVN:1.0\n";
if(out_align_file.size()>4){
if(out_align_file.compare(out_align_file.size()-4,4,".sam")==0) param.out_sam=1;
else if (out_align_file.compare(out_align_file.size()-4,4,".bam")==0) param.out_sam=2;
@@ -483,11 +486,19 @@ void RunProcess(void) {
}
if(param.out_sam&¶m.sam_header) {
+ int ret_len;
for(bit32_t i=0;i<ref.total_num;i++){
- sprintf(_ch,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
+ ret_len=snprintf(_ch,LINEBUFFSIZE,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
+ if (ret_len>=LINEBUFFSIZE) {
+ cerr<<"Buffer error, output was truncated (increase LINEBUFFSIZE in main.cpp and recompile)."<<endl;
+ }
_str.append(_ch);
}
- sprintf(_ch,"@PG\tID:BSMAP\tVN:%s\tCL:\"%s\"\n",version,command_line.c_str()); _str.append(_ch);
+ ret_len=snprintf(_ch,LINEBUFFSIZE,"@PG\tID:BSMAP\tVN:%s\tCL:\"%s\"\n",version,command_line.c_str());
+ if (ret_len>=LINEBUFFSIZE) {
+ cerr<<"Buffer error, output was truncated (increase LINEBUFFSIZE in main.cpp and recompile)."<<endl;
+ }
+ _str.append(_ch);
if(param.stdout) cout<<_str;
else if(param.pipe_out) fwrite(_str.c_str(),1,_str.size(),pout);
else fout<<_str;
From 3a1d8ff1b47416828d4a2687b4f1fab7bbdb2254 Mon Sep 17 00:00:00 2001
From: thomas <kreitler@molgen.mpg.de>
Date: Mon, 9 Oct 2017 15:39:33 +0200
Subject: [PATCH 2/3] main.cpp: Remove some whitespace clutter
so that the next commit will look cute :)
---
main.cpp | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/main.cpp b/main.cpp
index 761df2d..0f0b22d 100644
--- a/main.cpp
+++ b/main.cpp
@@ -562,18 +562,18 @@ void RunProcess(void) {
}
else fout.open(out_align_file.c_str());
}
-
+
if(param.out_sam&¶m.sam_header) {
- char _ch[1000];
- for(bit32_t i=0;i<ref.total_num;i++) {
- sprintf(_ch,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
- _str.append(_ch);
- }
+ char _ch[1000];
+ for(bit32_t i=0;i<ref.total_num;i++) {
+ sprintf(_ch,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
+ _str.append(_ch);
+ }
sprintf(_ch,"@PG\tID:BSMAP\tVN:%s\tCL:\"%s\"\n",version,command_line.c_str()); _str.append(_ch);
- if(param.stdout) cout<< _str;
- else if(param.pipe_out) fwrite(_str.c_str(),1,_str.size(),pout);
+ if(param.stdout) cout<< _str;
+ else if(param.pipe_out) fwrite(_str.c_str(),1,_str.size(),pout);
else fout<<_str;
- }
+ }
n_aligned=0;
info(1);
Do_SingleAlign();
From 2f18c2c1abac71fb28c286ab444941f01bd6224d Mon Sep 17 00:00:00 2001
From: thomas <kreitler@molgen.mpg.de>
Date: Mon, 9 Oct 2017 15:55:00 +0200
Subject: [PATCH 3/3] main.cpp: Add snprintf code in similar code section
Introduce snprintf & LINEBUFFSIZE in similar section for
'single-read alignments'. This should make the code a bit
cleaner.
---
main.cpp | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/main.cpp b/main.cpp
index 0f0b22d..9c252a2 100644
--- a/main.cpp
+++ b/main.cpp
@@ -564,12 +564,20 @@ void RunProcess(void) {
}
if(param.out_sam&¶m.sam_header) {
- char _ch[1000];
+ char _ch[LINEBUFFSIZE];
+ int ret_len;
for(bit32_t i=0;i<ref.total_num;i++) {
- sprintf(_ch,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
+ ret_len=snprintf(_ch,LINEBUFFSIZE,"@SQ\tSN:%s\tLN:%u\n",ref.title[i<<1].name.c_str(),ref.title[i<<1].size);
+ if (ret_len>=LINEBUFFSIZE) {
+ cerr<<"Buffer error, output was truncated (increase LINEBUFFSIZE in main.cpp and recompile)."<<endl;
+ }
_str.append(_ch);
}
- sprintf(_ch,"@PG\tID:BSMAP\tVN:%s\tCL:\"%s\"\n",version,command_line.c_str()); _str.append(_ch);
+ ret_len=snprintf(_ch,LINEBUFFSIZE,"@PG\tID:BSMAP\tVN:%s\tCL:\"%s\"\n",version,command_line.c_str());
+ if (ret_len>=LINEBUFFSIZE) {
+ cerr<<"Buffer error, output was truncated (increase LINEBUFFSIZE in main.cpp and recompile)."<<endl;
+ }
+ _str.append(_ch);
if(param.stdout) cout<< _str;
else if(param.pipe_out) fwrite(_str.c_str(),1,_str.size(),pout);
else fout<<_str;