Problem mit Hex2bix. (cypress Tool)

Thomas schrieb:

1

Outfilename=(char*)malloc(strlen(argv[i]+EXT_LENGTH));

Sicher dass es nicht

1

Outfilename=(char*)malloc(strlen(argv[i])+EXT_LENGTH);

sein sollte?

Ja ich bin mir sicher und das ist wohl auch das Problem .... Manchmal 
sieht man halt den Wald vor lauter Bäumen nicht. Der Code ist ja im 
Anhang.
Die Frage bleibt trotzdem warum geht sowas unter XP?
 Danke für den Hinweis.
Thomas

Ach du... das sollte man aber patchen...

1

[...]

2

3

case 'O':

4

  i++;

5

  OutFilename = (char *)malloc(strlen(argv[i]+EXT_LENGTH+1));

6

  strcpy(OutFilename, argv[i]);

7

  cont = FALSE;

8

  break;

9

10

[...]

11

12

if (OutFilename == NULL)

13

{

14

  root_length = strcspn(InFilename,".");

15

  OutFilename = (char *)malloc(root_length+EXT_LENGTH);

16

  memcpy(OutFilename,InFilename,root_length);

17

  OutFilename[root_length] = 0;

18

  strcat(OutFilename,Extension[OutFileType]);

19

}

20

21

[...]

22

23

Image = (BYTE *)malloc(MEMORY_BUFFER_SIZE);

24

ImageMap = (BYTE *)malloc(MEMORY_BUFFER_SIZE+2);

25

memset(Image,MEM_FILL,MEMORY_BUFFER_SIZE);

26

memset(ImageMap,FALSE,MEMORY_BUFFER_SIZE);

27

28

[...]

Soweit ich das beim ueberfliegen sehe, wird nie geprueft, ob man 
ueberhaupt speicher bekommen hat. Da gehoert zu jedem malloc noch ein

1

if (ptr == NULL) {

2

  // FEHLER! Mach was sinnvolles, z.B. Fehlerbehandlung...

3

}

Thomas schrieb:
> Die Frage bleibt trotzdem warum geht sowas unter XP?

Vielleicht gibt der Kernel unter XP größere Adressbereiche zurück sodass 
der dann folgende Buffer Overflow Fehler unentdeckt bleibt. So kann man 
sich natürlich beliebige (Sicherheits-)Probleme einfangen. Es "geht" 
also nur zufällig. Also froh sein dass Win10 hier strenger ist und 
solche Fehler auffallen...

Und sowas

1

memset(ImageMap,FALSE,MEMORY_BUFFER_SIZE);

2

                ^^^^^

ist auch ganz dolle pfui! Wenn man den Speicher mit 0 fuellen will, dann 
sollte man auch hinschreiben, dass man 0 meint, und nicht 'false'...

Der ganze Code sollte mal ueberarbeitet werden. Ist ja gruselig...

Kaj schrieb:
> Der ganze Code sollte mal ueberarbeitet werden. Ist ja gruselig...

So ist C doch immer - gehört zum guten Ton! Gute alte 
Programmier-Wertarbeit :-)

Das der Code nicht so dolle ist war mir schon klar. Um ehrlich zu sein 
habe ich mir das nie angeschaut da ja alles bisher funktioniert hat. Ich 
habe halt immer die exe von Cypress verwendet.
Das ganze ist mir ja erst aufgefallen als ich die Entwicklermaschine 
getauscht hatte.
Ich finde das ganze schon ziemlich interessant. Hätte cypress nicht den 
Quellcode offengelegt, hätte ich das Tool selber nachprogrammieren 
müssen.

Meine Frage wie ich das für mich verhindern kann hat sich aber erledigt.

Thomas

Auch das hier:

1

    if (compressIIC)

2

        while((ImageMap[addr] || ImageMap[addr+1] || ImageMap[addr+2]) && (addr < MemSize) && (i++ < 0x3ff))

3

            ++addr;

4

    else

5

        while(ImageMap[addr] && (addr < MemSize) && (i++ < 0x3ff))

6

            ++addr;

Was CppCheck dazu sagt:

1

Summary: Array index 'addr' is used before limits check.

2

3

Message: Defensive programming:

4

The variable 'addr' is used as an array index before it is checked that

5

is within limits. This can mean that the array might be accessed out of

6

bounds. Reorder conditions such as '(a[i] && i < 10)' to '(i < 10 && a[i])'.

7

That way the array will not be accessed if the index is out of limits.

Das dieses Programm jemals (ansatzweise) korrekt funktioniert hat grenzt 
an Zauberei...

Naja an dem Tool wurde sehr oft rumgeflickt. Ich habe aleine 4 Versionen 
davon und bin mir nicht sicher ab ich die aktuellste Version habe. Da 
ich nur eine Teilmenge der Optionen bisher genutzt habe waren mir 
Optionen wie das compressed iic bisher egal.
Bei Cypress ist da alles auf obsolet gestellt. Ich will gar nicht wissen 
wie viele Programme noch im Umlauf sind die ähnliche Probleme haben.
Thomas

Wenn man natürlich ein Programm unbedingt unportabel machen will... 
Außerdem können Pfade teilweise doch länger sein, siehe:

https://docs.microsoft.com/en-us/windows/desktop/fileio/naming-a-file

Das ist das schöne an "Undefined behaviour" (hier: das Schreiben auf 
nicht allokierten Speicher dank zu kleiner Allokation) - mal 
funktioniert's, mal stürzt es ab, mal produziert es klammheimlich 
falsche Ergebnisse, mal formatiert es deine Festplatte. Deswegen ist es 
wichtig, korrekte Programme zu schreiben, und nicht nur solche, die 
"bei mir ja funktionieren".

Christian R. schrieb:
> Seltsam, das scheint dann aber nicht immer aufzutreten

Das Problem existiert sobald man mit -O das Ausgabefile erstellt. Bei 
der automatischen Erzeugung ist alles okay

Thomas

Um das ganze abzuschließen im Anhang eine geänderte Version im Quelltext 
und compiliert
Sowie eine kleine Beschreibung dazu.
Es sind zwei Bugs behoben:
zum einen die Sache mit den Intel Hex checksum zum andern der Bug mit 
malloc bei -@O Filename.
Danke an alle die geholfen haben.

Thomas