LINUX.ORG.RU
решено ФорумTalks

Смеха ради: недостатки школокода

 , ,


0

1

Набрел на гитхабе, поделие юного фрилансера.

https://github.com/da411d/Crypto228/blob/master/crypto228.php

Собственно сабж. Какие у этого говнокода функции «шифрования дешифровки» строк есть явные, кричащие недостатки?

Понятное дело, что к рассмотрению не принимается тот очевидный факт, что это php и говнокод 18-20-летнего пацана :-)

Но вдруг мне когда-нить таким придется код ревью делать, а я не знаю к чему придраться :-D

★★★★★

Последнее исправление: Pinkbyte (всего исправлений: 2)
Ответ на: комментарий от pacify

ЕМНИП, MSDN + Google .

Такие вот были хеллоувордлы, сейчас даже немного стыдно, что не сподобился сваять чего-то лучшего)))

Был у меня, правда, тоже один алгоритм шифрования из книжки-самоучителя издательства «Диалектика», тоже скину сюда, если найду.

Twissel ★★★★★
() автор топика

справедливости ради, а что ты хотел от юного фрилансера?

чтоб он такой вжух и сразу херачил божественный код с паттернами и интерфейсами во все поля?

ckotinko ☆☆☆
()

Смеха ради на обсуждение надо свой код выставлять. Или хотя бы свои комментарии к нему, чтобы аудитория могла и по ним пройтись заодно.

А так это просто неэтично.

alpha ★★★★★
()
Последнее исправление: alpha (всего исправлений: 2)
Ответ на: комментарий от Twissel

ёпта, ты или сам ему объясни как или нафига ты критикуешь.

критиковать у нас каждый первый умеет. одних диванных аналитиков как собак нерезаных

ckotinko ☆☆☆
()
Ответ на: комментарий от ckotinko

Конкретно в этом случае я хотел научиться видеть не только очевидные недостатки в коде, которые всплывают на поверхность.

Поэтому и спросил, что я упустил, кроме того, что функция самопальная.

Twissel ★★★★★
() автор топика
Последнее исправление: Twissel (всего исправлений: 1)
Ответ на: комментарий от Twissel

тогда зачем ты нас об этом информируешь? мы тоже с ним незнакомы. может он нормальный чел, просто пока учится. потом напишет свой линукс будет уважаемый дядька в свитере с оленями и бородой, придет и откусит тебе голову.

ckotinko ☆☆☆
()
Ответ на: комментарий от ckotinko

Отписал выше.

Чисто чтобы уметь аргументированно доказывать при необходимости, что то-то и то-то не годится.

Twissel ★★★★★
() автор топика
Ответ на: комментарий от alpha

А так это просто неэтично.

В некоторой степени да, так и есть.

Но, во-первых, у него его Гитхаб-аккаунт в профиле в открытом доступе.

Во-вторых, никто не запрещал учиться «от противного»

В-третьих, я ничего плохого про чувака не сказал, даже наоборот, выше есть мой пост, где написано, что все в пределах разумного, для возраста, ВУЗа и рода занятий. Так что в данном случае я искал недостатки в коде, а не в его авторе.

Twissel ★★★★★
() автор топика
Ответ на: комментарий от ckotinko

свой линукс

А вот это вряд ли. Я видел как их учат «плюсам», там же в репе лабы есть)

Twissel ★★★★★
() автор топика
Ответ на: комментарий от Twissel

я ничего плохого про чувака не сказал

Ну-ну, «помогите мне высмеять школьника».

Когда хотят обсудить код - приносят код и обсуждают, а не выясняют, кто чей фрилансер. сколько ему лет и т.д. и т.п.

alpha ★★★★★
()
Ответ на: комментарий от alpha

«помогите мне высмеять школьника»

Ну и где у меня такое написано?

Женские педагогические домыслы, они такие домыслы :-)

Twissel ★★★★★
() автор топика
Ответ на: комментарий от Twissel

в целом кричащих ошибок нет.

быстро-решительно, вон из профессии, метлой махать.

Какие у этого говнокода функции «шифрования дешифровки» строк есть явные, кричащие недостатки?

1. байтодрочерство на высоком уровне -> прощай эффективность и скорость. много операций по строкам, замен и прочего. стоило бы выкинуть это в отдельный модуль и написать на каком-нибудь c/c++, и пользовать как библиотеку.

2. фактически, эта два куска абсолютно одинаковые (с точки зрения волшебного шифра XOR). нарушение DRY. надо всё переписать.

3. перец захаркожен, надо это в конфиг.

4. кроме тго, этот дятел делает замены символов. если в тексте уже будут символы равные заменяемым, то всё покарежится, что более чем вероятно. с точки зрения шифрования, это недопустимо. расшифрованный текст не будет совпадать с открытым.

5. шифр получился блочный. будет соблазн использовать его многопоточным образом. так что пусть обостнует тредо-безопасности используемых функций.

можно ещё легко несколько штук явных косяков найти (ибо нормально оценить цифр может только криптограф, но это видимо в задачу невходит). но мне лень, не пхп-щник ни разу.

n_play
()
Ответ на: комментарий от n_play

2.фактически, эта два куска абсолютно одинаковые (с точки зрения волшебного шифра XOR). нарушение DRY. надо всё переписать.

https://repl.it/repls/BluevioletWirelessMolecule

deadNightTiger, уже указал на это, на первой странице.

Я так понимаю, что XOR тут срабатывает из-за его (шифра) «побайтовости» ?

4. кроме того, этот дятел делает замены символов. если в тексте уже будут символы равные заменяемым, то всё покарежится, что более чем вероятно. с точки зрения шифрования, это недопустимо. расшифрованный текст не будет совпадать с открытым.

А вот этот косяк я упустил, спасибо :-)

Ну а вообще чувака не стоит судить строго, т.к. он еще студент.

Вон уже alpha меня осуждает за этот топик.

Twissel ★★★★★
() автор топика
Последнее исправление: Twissel (всего исправлений: 2)
Ответ на: комментарий от Twissel

зоро срабатывает из-за фундаментальных свойств своих.

a XOR b XOR b = a

4. кроме того, этот дятел делает замены символов. если в тексте уже будут символы равные заменяемым, то всё покарежится, что более чем вероятно. с точки зрения шифрования, это недопустимо. расшифрованный текст не будет совпадать с открытым.

а тут похоже я обосрался. там у него base64 стоит. посмотрел что это такое, там алфавит криптограммы получается урезанный. такой вариант развития событий исключен.

n_play
()
Ответ на: комментарий от deadNightTiger

вобщем в эти веб-говна не буду окунаться. но подвох в том месте мог быть.

Алфавит не урезанный

а разве не так? на входе произвольный бинарный открытый текст, на выходе обрезанный уже в конкретный алфавит?

n_play
()
Ответ на: комментарий от n_play

а разве не так? на входе произвольный бинарный открытый текст, на выходе обрезанный уже в конкретный алфавит?

Да, я неправильно тебя понял.

deadNightTiger ★★★★★
()
Ответ на: комментарий от Twissel

вобщем с точки зрения ревью - все мои подкопы - валидны. то что мне часть кода непонятно и потребует пояснений и ответов от автора, это нормально, так и должно быть в процессе.

и если непонятно будет нетолько мне, но и другим, то стоит либо переписать чтобы было понятнее. либо отмахнуться коментом с пояснением в коде.

n_play
()
Ответ на: комментарий от ggrn

Веб-розробник. Анімешник. Барабанщик.

хэээээдшоооооот!

targitaj ★★★★★
()
Ответ на: комментарий от pacify

Исходники 6-го класса, увы, не сохранились - это был ZX-совместимый «Дельта-С» чебоксарского завода «Элара».

батя зажопил денег на импортную японскую компакт-кассету?

n_play
()
Ответ на: комментарий от n_play

Блин, точно!

Ч — читабельность, я же ^ не заметил.

Ну тогда да, есть за что ругать.

Исключающего ИЛИ в коде я и не заметил, думал там какая-то другая закономерность срабатывает.

Ну тогда и MD5 абсолютно бесполезен для условного «укрепления» алгоритма. Действительно, как все запущено)))

Twissel ★★★★★
() автор топика
Ответ на: комментарий от Twissel

мне возможно показалось, но через мд5 он поперченый ключ разматывает до длинны открытого текста.

n_play
()
Ответ на: комментарий от pacify
unit Codec;
{Процедуры шифрования и дешифрования символов.
Процедура ввода пароля. Автор: Галисеев Г.В. Киев}
interface
   procedure setPassword;
   function coder(bt: Byte): Byte;
   function decoder(bt: Byte): Byte;
implementation
var
  charCounter: Byte;
  sum: Integer;
  arr: string; { Пароль }
procedure setPassword;
const
  numberChar = 6;
begin
  repeat
  Write('Insert password: ' ) ;
  Readln(arr);
  if Length(arr)<=numberChar then
  Writeln(' ** Length must be more than 6 characters**');
until Length(arr)>numberChar;
charCounter := 1;
sum := 0;
end;

function randomGenerator: Integer;
var j, cyrcleCounter: Byte;
begin
  cyrcleCounter := charCounter;
  for j:=0 to Length(arr)-2 do begin
  Inc(sum,Integer(arr[cyrcleCounter]));
  if cyrcleCounter=Length(arr) then
    cyrcleCounter := 0 ;
    Inc (cyrcleCounter);
end;
  Inc(charCounter);
  if charCounter=Length(arr) then
    charCounter := 1;
    if sum > 255 then
    Result := sum mod 255
  else
    Result := sum;
end;

function coder;
var
temp: Integer;
begin
  temp := bt - randomGenerator;
  if temp<0 then
    Result := Byte(temp + 256)
  else
    Result := Byte(temp);
end;

function decoder;
var
temp: Integer;
begin
  temp := bt + randomGenerator;
  if temp>255 then
    Result := Byte(temp - 256)
  else Result := Byte(temp);
end;
end. 

Вот первая процедура шифрования, с которой я ознакомился 12 лет назад :-)

Геннадий Галисеев Программирование в среде Delphi 7

Комментарий автора

Сам процесс шифрования заключается в искажении каждого байта в соответствии с псевдослучайной последовательностью, создаваемой на основе пароля. И ядром этого процесса является генератор псевдослучайных чисел. Этот генератор randomGenerator, да и весь алгоритм, я придумал сам, когда, читая о криптографии, я поразился несовершенству некоторых алгоритмов, которые можно было бы легко расшифровать, анализируя количество отдельных символов или находя закономерности. В данном алгоритме все основано на псевдослучайной последовательности и поэтому его нельзя расшифровать, если только не подобрать пароль. Но для этого нужно перебрать миллиарды комбинаций, ведь каждый символ пароля может принимать не менее ста значений (все буквы русского и английского алфавитов, цифры и знаки пунктуации), и даже при минимально разрешенном количестве символов 7 это будет 100 в степени 7

Сейчас это, конечно, выглядит как double facepalm, но тогда это была МАГИЯ!

Twissel ★★★★★
() автор топика
Последнее исправление: Twissel (всего исправлений: 2)
Ответ на: комментарий от Twissel

deadNightTiger,а какой тут фатальный недостаток?

Уточню, что это была учебная программа для шифрования файлов))

Twissel ★★★★★
() автор топика
Ответ на: комментарий от n_play

на импортную японскую компакт-кассету?

Откуда такие выводы?

Если ты помнишь то время, то наверное знаешь, что люди сдавали макулатуру, чтобы в пункте обмена получить кассету или прочие импортные ништяки. Это конец 80-х.

А кассеты для магнитофона вполне годились и обычные отечественные. Жаль только, выбросил я их в начале 2000-х.

pacify ★★★★★
()
Ответ на: комментарий от Twissel

Геннадий Галисеев Программирование в среде Delphi 7

Это всё понятно. А сейчас есть подобные журналы для юных программистов?

В начале 90-х ходили по рукам всякие самиздатовские журналы для программирования под Spectrum, БК, Вектор и прочими.

pacify ★★★★★
()
Ответ на: комментарий от ckotinko

Про шифроблокнот на лекциях Яндекса было, а гаммирование я как-то провтыкал.

P.S. А вроде как в шифроблокноте и используется гаммирование, только ЕМНИП гамма по длине должна быть равна открытому тексту.

Twissel ★★★★★
() автор топика
Последнее исправление: Twissel (всего исправлений: 1)
Ответ на: комментарий от pacify

Я ХЗ, может и нет, кроме Столярова в РФ.

То была книжечка в мягком переплёте.

Сейчас все больше видеокурсы, которые штампуют вебмакак.

Хотя может быть я ошибаюсь.

Twissel ★★★★★
() автор топика
Ответ на: комментарий от Twissel

Вот, в 1994 году для TASM (i286):

.model tiny
.286
.code
	org 100h
start:
	call BeepTimer
	int 20h
;-------------
BeepTimer proc
; cx = 1/18.2 sec
; dx = Hz
	mov al,0b6h
	out 43h,al
	mov al,0
	out 42h,al
	mov al,(1193180/330)/256
	out 42h,al
	in al,61h
	or al,3
	out 61h,al
	mov cx,18
	call Delay
	in al,61h
	and al,0fch
	out 61h,al
	ret
endp
;-----------
Delay	proc
; cx = 1/18.2 sec
	push ax
	push bx
	push dx
	xor ah,ah
	int 1ah
	mov bx,dx
	add bx,cx
delay_loop:
	int 1ah
	cmp dx,bx
	jne delay_loop
	pop dx
	pop bx	
	pop ax
	ret
endp
end start

pacify ★★★★★
()
Ответ на: комментарий от Twissel

А это уже 1996-й год, второй курс университета (MS DOS, Turbo Pascal):

{$E+}
uses Crt,Graph;

const X10  = -10;
      Y10  = 0;
      M1   = 100;
      V1X0 = 0;
      V1Y0 = 0;

      X20  = 10;
      Y20  = 0;
      M2   = 1;
      V2X0 = 0;
      V2Y0 = 10;

      TAU  = 0.01;

      rCX = 320;
      rCY = 175;
      rMX = 10;
      rMY = 10;

      cColor1 = White;
      cColor2 = White;
      cTrace = DarkGray;


var Gd,Gm :Integer;
    X1,Y1,X2,Y2 :Double;
    V1X,V1Y,V2X,V2Y :Double;
    XC,YC :Double;
    X1i,Y1i,X2i,Y2i :Integer;
var DX,DY,R2,F :Double;
    A1X,A1Y,A2X,A2Y :Double;

begin
     DetectGraph (Gd,Gm);
     InitGraph (Gd,Gm,'C:\TP\BGI');

     X1 := X10;
     Y1 := Y10;
     X2 := X20;
     Y2 := Y20;
     V1X := V1X0;
     V1Y := V1Y0;
     V2X := V2X0;
     V2Y := V2Y0;
     while not KeyPressed do
     begin
{       XC := (X1+X2)/2+Sin(X1/100)*100;
       YC := (Y1+Y2)/2+Sin(Y1/100)*100;
       PutPixel (Round(rCX+rMX*(X1-XC)),Round(rCY-rMY*(Y1-YC)), White);
       PutPixel (Round(rCX+rMX*(X2-XC)),Round(rCY-rMY*(Y2-YC)), LightGray);}
       X1i := Round(rCX+rMX*X1);
       Y1i := Round(rCY-rMY*Y1);
       X2i := Round(rCX+rMX*X2);
       Y2i := Round(rCY-rMY*Y2);
       PutPixel (X1i,Y1i, cColor1);
       PutPixel (X2i,Y2i, cColor2);
       X1 := X1 + V1X*TAU;
       Y1 := Y1 + V1Y*TAU;
       X2 := X2 + V2X*TAU;
       Y2 := Y2 + V2Y*TAU;

       DX := X2-X1;
       DY := Y2-Y1;
       R2 := Sqr(DX)+Sqr(DY);
       F  := 1;
       A1X := F*DX/M1;
       A1Y := F*DY/M1;
       A2X := -(F*DX/M2);
       A2Y := -(F*DY/M2);

       V1X := V1X + A1X*TAU;
       V1Y := V1Y + A1Y*TAU;
       V2X := V2X + A2X*TAU;
       V2Y := V2Y + A2Y*TAU;
       PutPixel (X1i,Y1i, cTrace);
       PutPixel (X2i,Y2i, cTrace);
     end;
     ReadKey;
     CloseGraph;
end.

pacify ★★★★★
()
Ответ на: комментарий от Twissel

Это 1999-й год, курсовая в университете:

procedure TGeneratorForm1.CalcEpsilon (matrix1 :TDoubleMatrix);
var i1,j1 :Integer;
    e1p, e2p :Double;
    e1m, e2m :Double;
    dt :Double;
begin
     // Make ep,em
     // One ... for matrix,q
     with matrix1 do begin
       for i1 := 1 to n1 do begin
         A[(i1-1)*n2+3] := 0; // e+ [3]
         A[(i1-1)*n2+4] := 0; // e- [4]
       end;

       for i1 := 1 to n1 do begin
         e1p := 1E+60; e2p := 1E+60;
         e1m := 1E+60; e2m := 1E+60;
         for j1 := 1 to n1 do begin
           if (j1 < i1) then begin
             dt := (A[(i1-1)*n2+1]-A[(j1-1)*n2+1]);
             if (A[(i1-1)*n2+2] <= A[(j1-1)*n2+2]) and (dt < e1p) then e1p := dt;
             if (A[(i1-1)*n2+2] >= A[(j1-1)*n2+2]) and (dt < e1m) then e1m := dt;
           end;
           if (i1 < j1) then begin
             dt := (A[(j1-1)*n2+1]-A[(i1-1)*n2+1]);
             if (A[(i1-1)*n2+2] <= A[(j1-1)*n2+2]) and (dt < e2p) then e2p := dt;
             if (A[(i1-1)*n2+2] >= A[(j1-1)*n2+2]) and (dt < e2m) then e2m := dt;
           end;
         end;
         if (e2p > e1p) then A[(i1-1)*n2+3] := e2p //p
         else A[(i1-1)*n2+3] := e1p;
         if (e2m > e1m) then A[(i1-1)*n2+4] := e2m // m
         else A[(i1-1)*n2+4] := e1m;
       end;
     end;
end;
....

pacify ★★★★★
()
Ответ на: комментарий от pacify

Я тоже посчитал. Но у меня TASM ругался, что мол значение слишком велико.

Может я ключ какой забыл при создании объектника, пофиг.

Когда просто 14 поставил, понятное дело, что скомпилировало.

Twissel ★★★★★
() автор топика
Ответ на: комментарий от Twissel

Я тоже посчитал. Но у меня TASM ругался, что мол значение слишком велико.

Возможно, я TASM.EXE брал из набора Turbo Pascal 5.5/6.0.

pacify ★★★★★
()
Последнее исправление: pacify (всего исправлений: 1)
Ответ на: комментарий от mandala

Почитал, это в криминальной жаргоне используется.

Pravorskyi ★★★
()
Ответ на: комментарий от Pravorskyi

И с uCoz никакой связи

Действительно, я с at.ua попутал.

Его употребление не является ошибкой

Употребление фамилий каких-то там красноармейцев тоже, почему ж их запретили?

bodqhrohro_promo
()
Ответ на: комментарий от bodqhrohro_promo

Употребление фамилий каких-то там красноармейцев тоже, почему ж их запретили?

Не понятно, причём тут слово «барабанщик», которое не запрещали.

Касаемо запрещения «фамилий каких-то там красноармейцев» — я не знаю, о ком идёт речь и кто там запрещал, но логично, что это вопросы к тем, у кого есть полномочия запрещать.

Pravorskyi ★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.