Re: [devel] [git update] packages/katrin: heads/master

["Slava Semushin" <slava.semushin@gmail.com>]

2007/7/12, Denis Klimov <zver / altlinux.org>:

Это опять я :)))

[...]
> --- a/src/db/db-service/katrin-db-service.c
> +++ b/src/db/db-service/katrin-db-service.c
> @@ -2,28 +2,39 @@
>  #include "katrin-db-service.h"
>  #include "db/dl-db.h"
>  #include "utils/utils.h"
> +
> +#define PROG katrin-db-service

Я не нашел где бы это использовалось. Если оно не используется, то
предлагаю удалить. А если и использовать, то как строку, в таком
случае katrin-db-service нужно заключить в кавычки, я думаю.

[...]
> + * katrin-db-service <action> <params>
> + *
> + * katrin-db-service clean YYYY-MM-DD YYYY-MM-DD [--nullcost]
> + * katrin-db-service sum YYYY-MM-DD YYYY-MM-DD <user|src_port|dst_port|src_addr|dst_addr|proto>
> + *
> + */
>  int main (int argc, char **argv) {
>         #include "conf/conf-katrin.inc"
>         #include "db/dl-db.inc"
> -       int find_all = 0;
> -       int i=0;
> -       for (i = 1; i < argc; i++) {
> -               if (strcmp(argv[i], "--clean") == 0)
> -                       find_all = 1;
> -               else
> -                       break;
> +       int find = 0;
> +       int num_rows = 0;
> +       if (strcmp(argv[1], "clean") == 0) {
> +               debug("enter into clean section");
> +               if (argc - 1 < 2) {
> +                       err("Usage with clean action: katrin-db-service clean YYYY-MM-DD YYYY-MM-DD [--nullcost]");
> +                       return 1;
> +               }
> +               num_rows = cleanstats(argv[2],argv[3]);
> +               info("DB clean: %d rows deleted",num_rows);
>         }
> -
> -
> -       if (argc - i != 2) {
> -               err("Two arguments required: a startdate and enddate. Date format YYYY-MM-DD [HH-MM-SS]\n");
> -               return 1;
> +       if (strcmp(argv[1], "sum") == 0) {
> +               debug("enter into sum section");
> +               if (argc - 1 < 3) {
> +                       err("Usage with sum action: katrin-db-service YYYY-MM-DD YYYY-MM-DD <user|src_port|dst_port|src_addr|dst_addr|proto>");
> +                       return 1;
> +               }
> +               num_rows = sumstats(argv[2],argv[3],argv[4]);
> +               info("DB sum: %d rows sum");
>         }
>
> -       int num_rows = cleanstats(argv[i],argv[i+1]);
> -
> -       info("DB Clean: %d rows deleted",num_rows);
> -
>         return 0;
>  }
>
> diff --git a/src/db/db.h b/src/db/db.h
> index 6cc318b..92d6caf 100644
> --- a/src/db/db.h
> +++ b/src/db/db.h
> @@ -16,6 +16,7 @@ void setblockuser (int userid);
>  void storestat (struct userTraffInfo traffik,int accuracy_stat);
>  struct filterslist *tariffid2filters (int tariffid);
>  int cleanstats(char *startdate, char *enddate);
> +int sumstats(char *startdate, char *enddate, char *opts);
>  struct userslist *tariffid2users(int tariffid);
>  #endif
>
> diff --git a/src/db/dl-db.h b/src/db/dl-db.h
> index 21fc5a1..bb15118 100644
> --- a/src/db/dl-db.h
> +++ b/src/db/dl-db.h
> @@ -22,6 +22,7 @@ void (*setblockuser)(int userid);
>  void (*storestat) (struct userTraffInfo traffik,int accuracy_stat);
>  struct filterslist *(*tariffid2filters) (int tariffid);
>  int (*cleanstats)(char *startdate, char *enddate);
> +int (*sumstats)(char *startdate, char *enddate, char *opts);
>  struct userslist *(*tariffid2users)(int tariffid);
>  #endif
>
> diff --git a/src/db/dl-db.inc b/src/db/dl-db.inc
> index 08fa47a..f952dac 100644
> --- a/src/db/dl-db.inc
> +++ b/src/db/dl-db.inc
> @@ -16,4 +16,5 @@ setblockuser = dlsym(handleLibDB,"setblockuser");
>  storestat = dlsym(handleLibDB,"storestat");
>  tariffid2filters = dlsym(handleLibDB,"tariffid2filters");
>  cleanstats = dlsym(handleLibDB,"cleanstats");
> +sumstats = dlsym(handleLibDB,"sumstats");
>  tariffid2users = dlsym(handleLibDB,"tariffid2users");
> diff --git a/src/db/libkatrin-db-mysql.c b/src/db/libkatrin-db-mysql.c
> index 6dfa17d..0871c92 100644
> --- a/src/db/libkatrin-db-mysql.c
> +++ b/src/db/libkatrin-db-mysql.c
> @@ -321,6 +321,7 @@ dbDisconnectMySQL(&mysqllink);
>  return ret_p;
>  }
>
> +// DB service functions
>  int cleanstats(char *startdate, char *enddate) {
>         debug("cleanstats");
>         MYSQL mysqllink;
> @@ -341,6 +342,43 @@ int cleanstats(char *startdate, char *enddate) {
>         return num_rows;
>  }
>
> +int sumstats(char *startdate, char *enddate, char *opts) {
[...]
> +       if (mysql_real_query(&mysqllink,query,(unsigned int) strlen(query)) !=0 ) {
[...]
> +                       if (mysql_real_query(&mysqllink,query,(unsigned int) strlen(query)) !=0 )
[...]

Здесь и в других файлах я вижу, что вы всегда приводите третий
аргумент ф-ции mysql_real_query() к типу unsigned int.

Вопрос: зачем?

strlen() возвращает тип size_t, который, насколько мне известно, на 32
битах и есть не что иное как unsigned int. Даже если и приводить
результат strlen() то уж точно не к unsigned int, а к unsigned long,
который и ожидает ф-ция mysql_real_query(), согласно описанию на
http://dev.mysql.com/doc/refman/5.0/en/mysql-real-query.html.

[...]

Вот ваш код:

>  int main (int argc, char **argv) {
[...]
> +       if (strcmp(argv[1], "sum") == 0) {
> +               debug("enter into sum section");
> +               if (argc - 1 < 3) {
> +                       err("Usage with sum action: katrin-db-service YYYY-MM-DD YYYY-MM-DD <user|src_port|dst_port|src_addr|dst_addr|proto>");
> +                       return 1;
> +               }
> +               num_rows = sumstats(argv[2],argv[3],argv[4]);
> +               info("DB sum: %d rows sum");
>         }
[...]

А теперь посмотрим на тело ф-ции sumstat():

> +int sumstats(char *startdate, char *enddate, char *opts) {
[...]
> +       sprintf(query,"select userid,sum(bytes),sum(cost) from stats WHERE storetime >= '%s' AND storetime <= '%s' group by userid;",startdate,enddate);
> +       debug("query: %s",query);
> +       if (mysql_real_query(&mysqllink,query,(unsigned int) strlen(query)) !=0 ) {
> +               err("MySQL select query error: %s",mysql_error(&mysqllink));
> +       }
[...]
> +}

Вы что-нибудь видите подозрительное в коде? Я да.

Например, использование sprintf() вместо более безопасного snprintf().
Но это цветочки... В данном коде допускается *ОЧЕНЬ* серьёзная ошибка
в безопасности. Вы используете в SQL-запросе данные, полученные от
пользователя, при этом не проверяя их!!!

Немного по теме: ru.wikipedia.org/wiki/Инъекция_SQL

Совет в данном случае: добавить ф-цию, к примеру, validate_date(),
которая бы получала указатель на строку и проверяла дата это или нет.
И потом везде где ожидается дата проверять аргументы. В качестве
примера можете посмотреть как проверяется дата в ф-ции
is_string_confirm_to_format() в OpenFM:

http://svn.berlios.de/viewcvs/openfm/trunk/src/common.c?rev=75&view=markup


P.S. BTW, char *startdate, char *enddate, char *opts в ф-ции
sumstats() не изменяются, а это значит что их неплохо было бы объявить
как const.


-- 
+ Slava Semushin | slava.semushin @ gmail.com
+ ALT Linux Team | php-coder @ altlinux.ru