Hot answers tagged

8

Things you did well on: Looks like you weren't too rusty based on this code. Looks very nice organizationally. You return different values for different error conditions. Things you could improve on: Standards Initialize i in your for loops.(C99) for (unsigned int i = 0; i < sizeof(digest); i++) Syntax Don't over-compact your code. if(argc != 6){...


8

C++ review You still need a C review! Idioms/Patterns RAII idiom In C++ we have this concept that an object should clean up its own resources. So when an object is created it will create and hold onto resources and when it is destroyed it will clean up those resources. When learning about the idiom people mostly talk about memory and smart pointers. ...


7

It would be nice to include a link to an authoritative reference. Until then, no comment on the algorithm compliance is possible. Instead of switch I'd rather have a table of strings indexed by BitcoinAddressState values. In C, main must return a value explicitly.


7

I would make the following changes, in order to make it more robust, and simpler: Do not use openssl and symmetric encryption. Use gpg and public key encryption. The advantage is that you do not have to store any passwords, anywhere (keep the secret key away from the backup script). Public key encryption is great for backups (you only need to access your ...


7

I'll start with a review of your current code before suggesting possible performance improvements. Various comments in your code do not add information and can be removed, for example //------------------------------Main Function------------------------// int main() free(string); //free string exit(EXIT_SUCCESS); //Program Ends Always use curly braces { ...


6

This is an incomplete answer; I'd just like to contribute more comments than easily fit in an actual comment. Consider replacing PASS="mysecretpasswordiwontwritehere" with PASS=`cat mysecretpassword.file` and moving your password into that file. Then you don't have to worry about always remembering to hide it when sharing your source code (or when editing/...


6

You can make these #defines: #define SHA1_DIGESTLENGTH 20 #define SHA1_BLOCK_LENGTH 64 #define COUNTER_LENGTH 8 into a more concise enum: typedef enum { COUNTER=8, SHA1_DIGEST=20, SHA1_BLOCK=64 } Length; This could also be done with SECRET and COUNTER, but each hex value would need a name. If you use the enum recommendation, you may need to rename the ...


6

Just a few notes on some things I didn't see mentioned. Compilation: I originally couldn't compile the program with the command in the comments. /tmp/cc2H2h0a.o: In function 'clc_pi': test.c:(.text+0x148): undefined reference to 'clock_gettime' test.c:(.text+0x2f0): undefined referenceto 'clock_gettime' collect2: ld returned 1 exit status Add -...


6

for crl in ${crls[@]}; do It's a good habit to always double-quote array expansions. Cleaning up the temp directory after a successful run would be a nice touch (or do away with it altogether; see below). cat ${crl_temp}/*.pem > ${crl_temp}/${new_crl} mv ${crl_temp}/${new_crl} ${crl_dir}/${new_crl} I assume you're doing this to get atomic replacement? ...


5

The documentation says that crypto_strong is set depending on the algorithm used. So it will never go from false to true, so yes, this might result in an infinite loop. Use it instead to see if you should trust the result. If it is true, the string should be secure. (you should also never design your own crypto algorithms for anything other than academic ...


5

On top on haneefmubarak's comment, I'd like to point out a couple of things I do not quite like : Variable names I think this : mpz_t v1, v2, v3, v4, v5; mpf_t V1, V2, V3, total, tmp, res; says it all. Not only the variables are meaningless names but on top of that, the case does matter. It's hard for everyone to read this without being confused. Also, ...


5

For starters, don't use __inline__. It isn't portable. Instead, use the standard C inline. Next, use more whitespace. Complex calls and almost all conditionals should go into multiple lines, while operations should be spaced. Also, explain what you are doing: // original snprintf(&(checksum[i*2]), 3, "%02x", (unsigned int)digest[i]); // what it ought ...


5

A buffer overflow: char HexResult[2*outputbytes-1]; Why the -1? There are 2 * outputbytes hex digits. So you need at least 2*outputbytes chars. Since you want a null terminated string (the rest of your code assumes it is), you even need an extra character for the \0 terminator. So this should be char HexResult[2 * outputbytes + 1]; A partial initialization:...


4

Portability The code would fail if the sender and receiver platforms disagree on the size of integer. The code would fail if the sender and receiver platforms disagree on the endianness of integer. Partial reads There is a chance that the very first read returns just a couple of bytes. The code still interprets it as a valid integer. Overall There is no ...


4

The worst problem with this code (IMO) is the readability, please take 10 minutes to have a look at other people's code on this site. I suspect you won't get much input because of the layout of your code. Name variables properly, you might know what I is for now, but in 6 months... You are deleting ssl and ctx without checking they have been allocated. You ...


4

Get rid of unnecessary nested and combined ifs While here it goes to no extreme, it's advisable to check for errors first, and to do that one by one - if possible, which is exactly this case. Do some extra checks Only in case of 0 arguments, the script should output usage message. This script does not support multiple arguments, so if given more than 1, ...


3

Looks good. No buffering problems anymore. Endianness is handled correctly. Reading and writing size is duly factored out. The only suggestion is not rely on the native unsigned type to represent the size. Notice that htonl takes uint32_t as a parameter. It is a strong hint to pass size as uint32_t as well. This guarantees that the size is 4 byte long no ...


3

You can use a here document to avoid all those calls to echo: cat <<END Usage: git-timestamp [options] command [revision] options -h - Show this usage info. All other options are ignored. -v - Output long revision instead of short. -l - Also show the local commit time of the specified revision. ...etc... END


3

Just a few minor things to add on top of what others already said. Prefer $(...) instead of `...` This is the modern way and it's easier to nest: DATE=$(date +%Y%m%d) No need to quote inside [[...]] This will work just fine: if [[ $LASTBACKUP == $DATE ]]; then echo "Do nothing, already backed up." exit fi And the 0 in exit 0 is redundant, as 0 is ...


3

There's no real way to speed up the brute-force enumeration of all possible 3-letter passwords. Perhaps you could use the dictionary. There's a finite list of 3-letter English words. They may be slightly more common. Also, if you google for "most common passwords", some kind of 3-letter version of that list could be tried before anything else.


3

I would like to know if is a good design A few points about your design from a c++ perspective: 1. Usage of free functions instead of classes From the point of OOP view, you should rather use classes than free functions to wrap / encapsulate the C API: namespace redi { namespace util { namespace base64 { class Encoder { public: Encoder(...


2

to answer your questions first: you don't need begin/end if it covers whole method, you can rescue directly from method, Net::HTTP already handles timeout, no need to build custom wrapper around it, now there are few other things that should be addressed: inline things when possible don't use @@ ever, use constants if you need to don't rescue from ...


2

GenerateRandomString is a poor name. I don't know what the contents of my string will be. In your case, you've restricted the character set to [0-9A-F], but that's not reflected in the name. Consider renaming this to generateRandomHexadecimalString or something that more clearly states the intention. You noted why $length is divided by two in a comment on @...


2

I've got a few things to say about the code, but having looked at the diff: - @ssl_version = "SSLv3" + @ssl_version = :auto and: - ctx.ssl_version = @ssl_version + ctx.ssl_version = @ssl_version unless @ssl_version == :auto your code changes seem minimal, and to the point doing exactly what they are supposed to do, and nothing more. ...


2

Might I suggest using OpenSSL's built in HMAC_* methods instead of re-rolling?


2

The best way to do space1="a b c d e f g h i j k l m n o p q r s t u v w x y z" for i in $space1; do for j in $space1; do for k in $space1; do echo $i$j$k done done done in bash is probably : for i in {a..z}{a..z}{a..z}; do echo $i; done Then, depending on what your homeworks says, it might or might not be what ...


2

Look at the man page for openssl http://www.openssl.org/docs/apps/openssl.html It is the command being executed here: $(openssl passwd -crypt -salt "$1" "$i$j$k") Another Clue below. Only roll your curser over it if you need the extra help. Still have not worked it out!


2

I honestly disagree with this whole approach. First, storing files IN a relational database is rarely a good strategy, limited almost exclusively to use cases where you want to perform binary searches against the binary artifacts. That is not your use case here. Outside of that, you are just adding a lot of overhead to your typical database management ...


2

Hardcoding /tmp/ssh.pub in doesn't look right. Makes the script vulnerable to all kinds of failures and race conditions. tempfile, perhaps? Similar concern applies to /tmp/pem.pub. Strongly recommend to derive pem name from $1. I am not sure I understand the significance of '1.2.840.113549.1.1.1'. Is there a reason to have a bash/python mixture (vs pure ...


2

In terms of error handling, it seems to be much safer to call ERR_clear_error() before any call to SSL_read(), SSL_write(), SSL_accept, and so on. See one of the few reported cases where error management has messed up people. Basically, each thread shares an error stack. While SSL_get_error() grab the latest error associated to a ssl session, it does not ...


Only top voted, non community-wiki answers of a minimum length are eligible